[Lagrangian.Solver] UnbuiltConstraintSolver: Fix resetForUnbuiltResolution (constraint re-ordering) #5871
+4
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a BeamAdapter scene where a catheter is deployed into deformable vessels, I got a huge discrepancy between LCPConstraintSolver (unbuilt) and UnbuiltGaussSeidelConstraintSolver (being the slowest).
Whereas the same scene (aka the standard scene 3instruments_collis where the catheter is deployed into static vessels, the difference is not that huge (~15-20% slower)
--> very important: wire_optimization must be true (more on that later)
Bench:
For reference WITHOUT wire_optimization (set to False)
We can see that wire_optimization seems to do nothing for UGS.
After investigation (thanks to Claude 🤖) it appeared that the re-ordering of the constraints was cancelled in the case of there is TWO constraint corrections (which is the case for the deformable vessels but not for the static vessels)
The loop calling resetForUnbuiltResolution on each ConstraintCorrection was resetting the ordering on the second iteration of the loop (e.g if the list is { LinearCC, UncoupledCC } then the first iteration will re-order fine but the second loop will reset the list and UncoupledCC do nothing for the re-oredering)
All in all the fix is easy, and set the initialization of the list outside the loop... 🤪
Results:
with the same ~15% difference as with the static vessel case.
✍️ Note that there should not be any difference between LCP and UGS but more to come about that....
[with-all-tests]
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if