New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SofaCore SofaConstraint] Update FreeMotionAnimationLoop so that it can compute a linearised version of the constraint force. #459

Merged
merged 68 commits into from May 30, 2018

Conversation

7 participants
@fjourdes
Copy link
Contributor

fjourdes commented Oct 9, 2017

This PR diffs against the sofa-framework::issofa_constrainsolving branch until it has been merged into master.

Objectives

  • Adjust FreeMotionAnimationLoop so that it is more faithful to the actual equations, most notably by
    allowing only one linearisation of the mappings within the time step.
  • Update the API in order to compute a linearised version of the constraint forces, in a similar fashion to
    what is done in the Compliant plugin.

Requirements

The following PR are required in order to build this

For testing:

  • SOFA_USE_MASK must be deactivated, otherwise derivatives quantities ( velocity at contact points for example) are not propagated at all since all dofs are "masked" by default. After having a rather quick look, apparently the SOFA_USE_MASK impose some strict (undocumented) requirements (on top of other limitations like the fact that it does not support changes in the topology) about where in the time step the "apply" method of mappings is called.

  • There is a branch on my local fork that integrates all the requirements for testing that can be found at the following location fjourdes:insimo_freemotion_integration

Changelog

API Breaking

[SofaCore]

  • ConstraintSolver can now define the VecId where the constraint lambdas and constraint correction are stored. Previously, these were fixed locations, usually VecId::externalForce() and VecId::dx(), but it could vary depending on the implementation of both the ConstraintSolver and the ConstraintCorrection API. Components can retrieve these locations when inspecting their ConstraintParams object which is usually a parameter of the API methods for constraint solving.
  • ConstraintParams contains the MultiMatrixDerivId that can be used by components related to the ConstraintSolver to retrieve the location of the constraint jacobian.
  • ConstraintResolution API constructor needs to be passed the number of lines that are involved in the solving of this constraint equation. The previous implementation was silently initializing this value to one. If that property has to modified because it depends on some other state variables of a concrete implementation of the ConstraintResolution, the setNbLines method has to be called to reflect it. Also added some getters/setters method.
  • BaseConstraintCorrection API separates the methods which computes the constraint displacement in motion space from the methods which apply it.
  • As result, factorized quite significantly the methods related to the ConstraintCorrection API
  • BaseConstraint API defines a storeLambda method which is used to store the result of the constraint force computation (stored in a BaseVector form) inside a dedicated state vector of a MechanicalState. Requires PR #456. This value can be used later on to discover the stiffness coming from the non linear mapping that result from it.
  • BaseMechanicalState resetConstraint and getConstraintJacobian methods need to be given a ConstraintParams to be able to retrieve the MatrixDerivId containing the constraint jacobian.
  • LinearSolver API
    • rename applyContactForce API method into applyConstraintForce. This method is no longer responsible for applying the corrective motion, but just to compute it.
    • buildComplianceMatrix API method needs to be passed a ConstraintParams so that concrete instances of LinearSolver can retrieve the MatrixDerivId that points to the constraint jacobian.

FIX

[SofaConstraint]

  • GenericConstraintSolver block gauss seidel can support arbitrary size of blocks. The previous implementation had an hard coded limit of 6 for the block size.

UPDATES

[SofaSimulationCore]

  • Adapted AnimateVisitor, MechanicalGetConstraintJacobianVisitor and MechanicalResetConstraintVisitor to the changes introduced in BaseMechanicalState API

[SofaBaseMechanics]

  • MechanicalObject dynamically allocated state vectors are given a name and a owner like any other Data. As a result, these dynamic state vectors can be displayed by the GUI. Also adapted the code to the API changes introduced in BaseMechanicalState API

[SofaMeshCollision]

  • Some contact mappers updateXfree() method were not calling applyJ and therefore were not propagating the free velocity towards contact dofs, resulting in segfault when solving constraints in velocity mode.

[SofaConstraint]

  • GenericConstraintSolver accumulates the constraint lambdas towards independant dofs, so that non linear mappings can compute the geometric stiffness induced by the constraint forces from the previous time step.
  • FreeMotionAnimationLoop assembles mapping geometric stiffness when possible. It also uses a single mapping linearisation within the time step. The solving step becomes very similar to the one explained in the "Stable Constraints" paper, when the compliance of the constraint is equal to zero.
  • BilateralConstraintResolutionNDofs uses Eigen library to factorize a constraint block of arbitrary size using LDLT decomposition.
  • ConstraintAnimationLoop minor changes introduced to reflect API changes.

[SofaMiscMapping]

  • DistanceMapping implements the applyJT method for MatrixDeriv types using the utility methods provided in #452

[SofaGeneralAnimationLoop]

  • MultiStepAnimationLoop and MultiTagAnimationLoop : adapted code to reflect API change.

ADD

[SofaConstraint]

  • MappingGeometricStiffnessForceField can assemble the geometric stiffness of a mapping.
  • UniformConstraint defines a constraint with a uniform direction in the contact space, internally it represents an identity matrix. It computes the right hand side of constraint equations in velocity mode as rhs = phi / dt + dvfree, following the same calculus notation and derivation as the one explained in the "Stable Constraints" paper. In position mode the constraint rhs differs from the constraint rhs in velocity mode by a dt factor. rhs = phi + dvfree * dt which gives rhs = dxfree which is similar to what was done by the previous implementation.
  • ConstraintStoreLambdaVisitor which is used by the GenericConstraintSolver to store the lambda resulting from the constraint solving step into a specific location ( by default VecId::externalForce(), but a dynamic VecId can be used also)

[Examples]

  • InextensiblePendulum.scn shows the benefits of the linearisation of the constraint force

Remarks

  • MappingGeometricStiffnessForceField suffers the same limitation as any other forcefield, ie the mapping must be directly connected to the independant dofs, otherwise it would require an additional unsupported computation to project the mapped stiffness matrix into the space of the independant degrees of freedom. Multimappings support is not there, since it would probably require some adapatation in the API, so that it is easy to retrieve the stiffness block associated with each of the mapping parent dofs.

  • ContactMappers : It is (very) questionable to let contact mappers propagate the unconstrained dynamics solution vector towards contact dofs by calling updateXfree() since it will induce inconsistencies in mapping linearisation if the contact mappers are non linear, since contact mappings will therefore be linearised around the free motion configuration, and the rest being linearised around the configuration at the beginning of the time step. This PR does not address this problem which is left for future work. It arises only with non linear contact mappers (like the RigidContactMapper for instance), but this should be kept in mind.

  • LinearSolver API : As a general note, I believe the LinearSolver API is a bloated with virtual methods which are difficult to understand since they are not directly related to the computation / factorisation of a positive definite matrix, and are mostly optional especially for non assembled solver like CG. Most of the methods should reside in the ConstraintCorrection API, and concrete instance of ConstraintCorrection should be templated on the type of LinearSolver in my opinion, since it is the only objects that actually make use of these methods, and really what you want is to have access to the concrete type of Matrix and Vector used by the solver. Also from my understanding only LinearSolver instances that derive from MatrixLinearSolver implement these kind of methods, so maybe a first step would be to move them here.


This PR:

  • builds with SUCCESS for all platforms on the CI.
  • does not generate new warnings.
  • does not generate new unit test failures.
  • does not generate new scene test failures.
  • does not break API compatibility.
  • is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

@fjourdes fjourdes changed the title Insimo freemotion Update FreeMotionAnimationLoop so that it can compute a linearised version of the constraint force. Oct 9, 2017

@fjourdes fjourdes changed the title Update FreeMotionAnimationLoop so that it can compute a linearised version of the constraint force. [SofaCore SofaConstraint] Update FreeMotionAnimationLoop so that it can compute a linearised version of the constraint force. Oct 9, 2017

@fjourdes

This comment has been minimized.

Copy link
Contributor

fjourdes commented Oct 12, 2017

I believe that if that branch makes it way to master, someone at mimesis should have close look too.

@guparan guparan referenced this pull request Oct 19, 2017

Merged

[all] issofa_constraintsolving: improve constraints #484

6 of 6 tasks complete
@guparan

This comment has been minimized.

Copy link
Member

guparan commented Oct 19, 2017

I am ready to rebase your branch on issofa_constraintsolving (updated with master). Are you OK for me to force push?

@ChristianDuriez

This comment has been minimized.

Copy link
Contributor

ChristianDuriez commented Oct 19, 2017

You will do it in a branch of Master ? I was suppose to push it on sofaDefrost but if you are doing it on the Master, that's even better...

@fjourdes

This comment has been minimized.

Copy link
Contributor

fjourdes commented Oct 19, 2017

@guparan : I think it is fine to rebase with the updated issofa_constraint_solving branch. I believe the conflicts come from the override keyword that was added in many places in the code recently.
The integration branch fjourdes:insimo_freemotion_integration will still work if someone wants want to try.

@guparan guparan force-pushed the fjourdes:insimo_freemotion branch from 79f4f82 to 6a97fe9 Oct 19, 2017

@guparan

This comment has been minimized.

Copy link
Member

guparan commented Oct 19, 2017

It's done. Yes most of the conflicts went from override keywords.
@ChristianDuriez I did it on this branch to be able to easily rebase it on master when #484 will be merged.

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented Jan 3, 2018

This PR will be rebased and reviewed this week.
Any help in review would be most welcome @EulalieCoevoet ;)

@sofa-framework sofa-framework deleted a comment from sofabot Jan 3, 2018

Matthieu Coquet and others added some commits May 26, 2017

UPDATE: FreeMotionAnimationLoop. Add option to keep the linearisation of
the mappings around the freemotion. It is disabled by default, so no
change is actually introduced from the exisiting behavior.
UPDATE: SofaCore. ConstraintSolver API. Make it possible for derived
classes to redefine the state vector allocation for the constraint
lambdas and the corrective displacements.
WARNING CHANGE: SofaCore ConstraintCorrection API.
- ConstraintParams need to be given the VecId where the constraint
  jacobian must be accumulated, and where the constraint lambda must be
  stored. Right now the current API mostly assumes that the constraint
  lambda and the constraint force are stored in pre defined VecIds. It
  was therefore difficult to know where these quantities are stored,
  because it was dependent both on the constraint solver, and the
  constraint correction scheme used in the scene.

- Separate method which computes the corrective displacement coming
  from the constraint force from the methods which apply this corrective
  displacement. This is anyway what was silently done in the concrete
  implementations of the ConstraintCorrection API.
WARNING CHANGE: SofaCore. LinearSolver API
- give read only access to the MultiMatrixAccessor used to construct
  the system matrix, if the solver actually builds it.

- rename applyContactForce method into applyConstraintForce method.

- also change the signature of the method, and document its
  responsibility, which is bounded to:

  -- projecting the constraint force in the motion space provided the
  constraint jacobian is known (which should be the case at the time
  this method is called)

  -- computing dx = A^-1 . J^t . lambda
     and storing the result in the VecId passed as a parameter.
CHANGE: SofaSimulation MechanicalIntegrateConstraintsVisitor
The visitor must be passed the location of the VecId where the
corrective motion coming from the influence of the constraint force is
stored. On overall this Visitor can be tuned to work along with the
parameters of the methods of the ConstraintCorrection API.
UPDATE: SofaConstraint. Reflect changes in ConstraintCorrection API.
Also, PrecomputedConstraintCorrection can no longer rely on the former
API methods to compute the list of "active dofs" (which correponds to
the sparsity pattern of the constraint jacobian). Instead this list is
computed whenever the update of the compliance matrix is done.
UPDATE: SofaConstraint. FreeMotionAnimationLoop constructs a
ConstraintParams object that contains the necessary information to
retrieve:

- where to read the free motion of the dofs ( as done before )

- where the constraint jacobian must be accumulated

- where the constraint force must be stored
UPDATE: SofaConstraint. Update GenericConstraintSolver with changes
introduced in ConstraintCorrection API. It also allocates dynamic VecId
for constraint jacobian and constraint lambda.
UPDATE: [SofaConstraint]. UniformConstraint can use a constraint reso…
…lution which factors the compliance block to perform a direct solving of the constraint blocK. The default mode stays the iterative one where each line is treated one after the other.
ADD: [SofaConstraint]. UniformConstraint, a component that shares sim…
…ilarities with UniformCompliance from the Compliant plugin. The idea is to be able to express constraint equations compatible with the Constraint / ConstraintCorrection API with mappings that give a strain measure and its jacobian.
@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented Apr 16, 2018

[ci-build][with-scene-tests]

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 9, 2018

test BilateralInteractionConstraint_test failing due to the constraint which is not exactly met

@hugtalbot hugtalbot added STC#5 and removed STC#5 labels May 9, 2018

@ChristianDuriez

This comment has been minimized.

Copy link
Contributor

ChristianDuriez commented May 10, 2018

Hi,
I think I have solved the test problem.. by changing the test !
It was not designed to take into account the tolerance of the solver...
see commit:
fjourdes@78b8459

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 11, 2018

Hi Christian,

Using a tolerance is working, but is this normal that the constraint is not exactly applied? (although it was before)

@damienmarchal

This comment has been minimized.

Copy link
Contributor

damienmarchal commented May 15, 2018

[ci-build][with-scene-tests]

@damienmarchal

This comment has been minimized.

Copy link
Contributor

damienmarchal commented May 16, 2018

@hugtalbot , @ChristianDuriez Can you reproduce locally the problem that happens in ubuntu or is this a CI issue ?

@ChristianDuriez

This comment has been minimized.

Copy link
Contributor

ChristianDuriez commented May 16, 2018

I am not on Ubuntu. It seems to be a problem with a test in the Flexible plugin.

FIX one crash and the added warnings
One scene was crashing in SensableEmulation plugin : testOmniDriverEmu.scn
I fixed it by adding the option : solveVelocityConstraintFirst="true"

Fixed added warnings
- in GenericConstraintSolver.cpp line 368
none of the function parameter (cParams, res1 and res2) were unused.
- in GenericConstraintCorrection.cpp line 277
the complianceFactor is not used.
@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 18, 2018

@ChristianDuriez
One scene was crashing in SensableEmulation plugin : testOmniDriverEmu.scn
I fixed it by adding the option : solveVelocityConstraintFirst="true". I have no idea why actually! Could you give some insight?

I fixed some added warnings, let me know if this was normal:
Note that :

  • in GenericConstraintSolver.cpp line 368 : none of the function parameter (cParams, res1 and res2) were unused.
  • in GenericConstraintCorrection.cpp line 277 : the complianceFactor is not used

What appears really necessary, is to have some documentation on this constraint pipeline. This could be a good task for the STC#5 don't you think?

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 18, 2018

[ci-build][with-scene-tests]

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 28, 2018

Conflicts are again fixed, if it compiles the PR will have to be merged asap

@hugtalbot

This comment has been minimized.

Copy link
Contributor

hugtalbot commented May 29, 2018

[ci-build][with-scene-tests]

@damienmarchal

This comment has been minimized.

Copy link
Contributor

damienmarchal commented May 30, 2018

Merge ...

@hugtalbot hugtalbot merged commit 8667226 into sofa-framework:master May 30, 2018

6 checks passed

Dashboard Builds triggered.
Details
Scene tests Triggered in latest build.
Details
centos_clang-3.4_options OK (tests ignored, see details)
Details
mac_clang-3.4_options OK (tests ignored, see details)
Details
ubuntu_gcc-5.4_options OK (tests ignored, see details)
Details
windows7_VS-2015_options_amd64 OK (tests ignored, see details)
Details

@guparan guparan added this to the v18.06 milestone Jun 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment