Physics architecture refactoring
11 views
Skip to first unread message
Julien Lenoir
Mar 21, 2014, 10:24:30 AM3/21/14
to openSurgSim
Hi OSS-users,
the current physics architecture presents many code duplication
between mass-spring and fem and a common architecture including
rigid-body simulation was always part of the internal discussions. We
will try to address these issues in a near future.
As a effort toward this, here is a snippet introducing what the
architecture could look like:
https://www.assembla.com/spaces/OpenSurgSim/snippets/11013
This proposal has many advantages and knock off many features we
wanted to achieve in OSS:
* Common interface/core calculation for all physics representations.
=> refactor RigidRepresentation to fit this architecture.
Mass-spring and Fem refactoring will be less painful as they already
follow a very similar core calculation process.
* Split mass and deformation. API-wise, it helps to not break any
physics concept. A mass element has no deformation property and a
deformation element has no mass property, especially for mass-spring
and rigid-body. A fem element will simply combine both properties.
* Unique state for all representations.
* Concept of ExternalForce. This concept exist currently for the VTC
in a more hard-coded way in the RigidRepresentation. This introduces a
generalization that can be applied for any force on any representation
(gravity, rayleigh damping, vtc, ...)
* Give control of the boundary conditions to the OdeSolver, they
should be applied on the proper structure and this will help staying
consistent no matter what solver we choose.
This proposal also underline the generalized concept of forces and how
they are gathered cleanly as internal/external forces in the
Representation.
Thoughts, comments are welcome.
-- Julien
the current physics architecture presents many code duplication
between mass-spring and fem and a common architecture including
rigid-body simulation was always part of the internal discussions. We
will try to address these issues in a near future.
As a effort toward this, here is a snippet introducing what the
architecture could look like:
https://www.assembla.com/spaces/OpenSurgSim/snippets/11013
This proposal has many advantages and knock off many features we
wanted to achieve in OSS:
* Common interface/core calculation for all physics representations.
=> refactor RigidRepresentation to fit this architecture.
Mass-spring and Fem refactoring will be less painful as they already
follow a very similar core calculation process.
* Split mass and deformation. API-wise, it helps to not break any
physics concept. A mass element has no deformation property and a
deformation element has no mass property, especially for mass-spring
and rigid-body. A fem element will simply combine both properties.
* Unique state for all representations.
* Concept of ExternalForce. This concept exist currently for the VTC
in a more hard-coded way in the RigidRepresentation. This introduces a
generalization that can be applied for any force on any representation
(gravity, rayleigh damping, vtc, ...)
* Give control of the boundary conditions to the OdeSolver, they
should be applied on the proper structure and this will help staying
consistent no matter what solver we choose.
This proposal also underline the generalized concept of forces and how
they are gathered cleanly as internal/external forces in the
Representation.
Thoughts, comments are welcome.
-- Julien
Paul Novotny
Mar 21, 2014, 11:32:43 AM3/21/14
to opens...@simquest.com
To add some info, I was doing some simple performance tests with the FEM
in the stapling demo. I found a couple things that could simplify
physics:
1) There was no noticeable difference (ie, within +- 0.01s) between
calling computeFMDK and calling computeF, computeM, computeD, computeK
separately. So I think that computeFMDK is not needed.
2) More interestingly. The Explicit Euler solver only needs to call
computeM and computeF. But I tried just adding computeD and computeK
calls and there was no noticeable difference in speed.
3) And finally, the only place where a non-dense matrix is used is for
the mass matrix of the MassSpringRepresentation. Switching the mass
matrix to dense in the OdeEquation had no effect on speed. Using a dense
matrix in the OdeSolver, did however cut the speed in half.
These results suggest that building the M,K,D,F matrices are an
insignificant part of the solving the FEM, even if they are dense. So I
suggest we do a couple of things. First, I think a
DeformableRepresentation should no longer 'be an' OdeEquation, but 'have
an' OdeEquation. The OdeEquation could basically be a struct of dense
matrices (F,M,D,K). They would be built regardless of the OdeSolver.
These steps will decouple OdeEquation and OdeSolver and remove the
templatization of the DeformableRepresentations.
But, I would like to keep the templatization of OdeSolver. The matrix
type is very important in the solver, especially when you start
inverting matrices. So the MassSpringRepresentation might look something
like this now:
public MassSpringRepresentation : public DeformableRepresentation
{
...
private:
OdeEquation m_odeEquation;
OdeSolver<Diagonal, Dense, Dense, Dense> m_odeSolver;
}
Finally, this change simplifies the elements (Springs, Masses,
FemElements). Now instead of asking a spring or FemElement to add it's
force, and damping (addForce, addDamping). You can just call:
element->updateEquation(m_odeEquation)
and that is all the interface a spring, mass, and FemElement needs to
provide. This change reverses the current approach and Julien's
suggestion. The current structure is built around the OdeSolvers only
getting what information they need, so they ask for the OdeEquation for
the mass matrix which in turn asks the elements for their mass matrices.
But, this results in a lot of complexity in the design. The design I am
suggesting is simpler, but the OdeSolvers get everything regardless if
they need it (with a hopefully negligible performance hit).
-Paul
in the stapling demo. I found a couple things that could simplify
physics:
1) There was no noticeable difference (ie, within +- 0.01s) between
calling computeFMDK and calling computeF, computeM, computeD, computeK
separately. So I think that computeFMDK is not needed.
2) More interestingly. The Explicit Euler solver only needs to call
computeM and computeF. But I tried just adding computeD and computeK
calls and there was no noticeable difference in speed.
3) And finally, the only place where a non-dense matrix is used is for
the mass matrix of the MassSpringRepresentation. Switching the mass
matrix to dense in the OdeEquation had no effect on speed. Using a dense
matrix in the OdeSolver, did however cut the speed in half.
These results suggest that building the M,K,D,F matrices are an
insignificant part of the solving the FEM, even if they are dense. So I
suggest we do a couple of things. First, I think a
DeformableRepresentation should no longer 'be an' OdeEquation, but 'have
an' OdeEquation. The OdeEquation could basically be a struct of dense
matrices (F,M,D,K). They would be built regardless of the OdeSolver.
These steps will decouple OdeEquation and OdeSolver and remove the
templatization of the DeformableRepresentations.
But, I would like to keep the templatization of OdeSolver. The matrix
type is very important in the solver, especially when you start
inverting matrices. So the MassSpringRepresentation might look something
like this now:
public MassSpringRepresentation : public DeformableRepresentation
{
...
private:
OdeEquation m_odeEquation;
OdeSolver<Diagonal, Dense, Dense, Dense> m_odeSolver;
}
Finally, this change simplifies the elements (Springs, Masses,
FemElements). Now instead of asking a spring or FemElement to add it's
force, and damping (addForce, addDamping). You can just call:
element->updateEquation(m_odeEquation)
and that is all the interface a spring, mass, and FemElement needs to
provide. This change reverses the current approach and Julien's
suggestion. The current structure is built around the OdeSolvers only
getting what information they need, so they ask for the OdeEquation for
the mass matrix which in turn asks the elements for their mass matrices.
But, this results in a lot of complexity in the design. The design I am
suggesting is simpler, but the OdeSolvers get everything regardless if
they need it (with a hopefully negligible performance hit).
-Paul
Harald Scheirich
Mar 24, 2014, 7:54:44 AM3/24/14
to openSurgSim
Overall this looks reasonable, I would second not deriving from OdeEquation but containing OdeEquation, what I don't quite get is why we have two parallel interfaces Rigid and Deformable when all Deformables implement the rigid interface as well (i.e. why do multiple inheritance rather than just plain), if we stick to fully virtual inheritance in our multiple inheritance then things will usually work out, if we start inheriting implementation things get a little bit hairy sometimes.
My main concern is the exposure of private information in the algorithms and the code, e.g. code that uses the state in a way that depends on the internal structure of the state, or code that manipulates the OdeEquation by pulling indeces from the item that is is dealing with. If we do a big refactoring like this I would wish for more encapsualtion of the 'private' data. The example I currently give is
void Constraint::build(double dt,
MlcpPhysicsProblem* mlcp,
unsigned int indexOfRepresentation0,
unsigned int indexOfRepresentation1,
unsigned int indexOfConstraint)
except for dt, and possibly mlcp, the other indices should be know to the constraint, the representations are both part of the constraint and it should probably know its own index, passing this information from the outside is a source of error but also tends to aggregate lists of indices (list of indices of all the contraints) on the outside that parallel the list of things (list of all the contraints)
So I gather the reasoning for this refactoring is to reduce duplication of code am I correct ? How does this mesh with improving the performance of the FEM ? I would like us to focus rather on one thing than to move sideways with this refactoring and try to squeeze in the performance work. Performance of the FEM is actually blocking us right now.
Is there a way to do this in stages rather than having to deal with all the physics pieces in one go ?
Harry
--
You received this message because you are subscribed to the Google Groups "openSurgSim" group.
To unsubscribe from this group and stop receiving emails from it, send an email to opensurgsim...@simquest.com.
To post to this group, send email to opens...@simquest.com.
Visit this group at http://groups.google.com/a/simquest.com/group/opensurgsim/.
Julien Lenoir
Mar 24, 2014, 9:26:53 AM3/24/14
to Harald Scheirich, openSurgSim
The discussion on physics refactoring is for future work and do not
need to happen now. I just wanted to open the discussion so that when
we have more time for refactoring, we know which direction to go. I
agree with Harry that performance is a big problem right now and it
needs to be addressed urgently. The bigger physics refactoring can
happen later.
My two cents so far on the discussion:
* [Harry] The constraint receives the indices because we wanted
precisely the Representation to not have the indices (in our old code
base, the Representation had their own index). We just need to keep in
mind that these indices are not fixed and are dynamically assigned by
the PhysicsManager when processing the constraints (this will be very
useful when we will introduce Island type architecture in physics). So
far, we kept that information on the PhysicsManager level and this is
why it is passed on to the Constraint and not coming from the
Representation.
Potential problem that can arise from displacing the indices:
If the RepresentationIndex is pushed in the Representation, the
problem that can occur is that an MLCP build with a set of index is
backed-up or passed on to the Fast-Haptic, and the indices are not
part of it anymore but rather known by the Representation. If the
simulation keeps running and a new constraint configuration arises in
the scene, the PhysicsManager will re-assign the indices accordingly,
modifying the index with which our MLCP was build.
These indices are MLCP related (maybe a renaming would be in order ?)
and should stay close to the MLCP. The PhysicsManagerState was a good
spot to have them, right along with the MLCP. If they have to move,
they should go in the MlcpPhysicsProblem itself.
* [Paul] 1) If I understood properly, you see a 10ms time difference
between calling addFMDK and calling the individual methods. That is
actually very big in physics simulation. Keep in mind that the time
step is 1ms, so the real time taken to solve 1 time step should be as
close to 1ms as possible. In light of that, switching to calling the
individual methods will make us 10 times slower than we should already
be.
Even if we change the time step to 10ms, that means only calling the
individual methods is already chewing up all our time, not counting
the linear system resolution.
* [Paul] 2) What do you call not noticeable ?
* [Paul] 3) If switching from a diagonal matrix to a dense matrix has
no effect in time, I would argue we should drop Eigen for something
else ! I suspect, from your 2 comments, that we are doing the wrong
thing somewhere...and we should look into it.
Performance is a crucial factor in real time simulation and we need to
focus on it more. So any future refactoring that could potentially
make performances worst should be thought twice.
Finally, these refactoring can certainly happen in stages.
-- Julien
need to happen now. I just wanted to open the discussion so that when
we have more time for refactoring, we know which direction to go. I
agree with Harry that performance is a big problem right now and it
needs to be addressed urgently. The bigger physics refactoring can
happen later.
My two cents so far on the discussion:
* [Harry] The constraint receives the indices because we wanted
precisely the Representation to not have the indices (in our old code
base, the Representation had their own index). We just need to keep in
mind that these indices are not fixed and are dynamically assigned by
the PhysicsManager when processing the constraints (this will be very
useful when we will introduce Island type architecture in physics). So
far, we kept that information on the PhysicsManager level and this is
why it is passed on to the Constraint and not coming from the
Representation.
Potential problem that can arise from displacing the indices:
If the RepresentationIndex is pushed in the Representation, the
problem that can occur is that an MLCP build with a set of index is
backed-up or passed on to the Fast-Haptic, and the indices are not
part of it anymore but rather known by the Representation. If the
simulation keeps running and a new constraint configuration arises in
the scene, the PhysicsManager will re-assign the indices accordingly,
modifying the index with which our MLCP was build.
These indices are MLCP related (maybe a renaming would be in order ?)
and should stay close to the MLCP. The PhysicsManagerState was a good
spot to have them, right along with the MLCP. If they have to move,
they should go in the MlcpPhysicsProblem itself.
* [Paul] 1) If I understood properly, you see a 10ms time difference
between calling addFMDK and calling the individual methods. That is
actually very big in physics simulation. Keep in mind that the time
step is 1ms, so the real time taken to solve 1 time step should be as
close to 1ms as possible. In light of that, switching to calling the
individual methods will make us 10 times slower than we should already
be.
Even if we change the time step to 10ms, that means only calling the
individual methods is already chewing up all our time, not counting
the linear system resolution.
* [Paul] 2) What do you call not noticeable ?
* [Paul] 3) If switching from a diagonal matrix to a dense matrix has
no effect in time, I would argue we should drop Eigen for something
else ! I suspect, from your 2 comments, that we are doing the wrong
thing somewhere...and we should look into it.
Performance is a crucial factor in real time simulation and we need to
focus on it more. So any future refactoring that could potentially
make performances worst should be thought twice.
Finally, these refactoring can certainly happen in stages.
-- Julien
Paul Novotny
Mar 24, 2014, 10:03:38 AM3/24/14
to opens...@simquest.com
On Mon, 2014-03-24 at 10:26 -0300, Julien Lenoir wrote:
> * [Paul] 1) If I understood properly, you see a 10ms time difference
> between calling addFMDK and calling the individual methods. That is
> actually very big in physics simulation. Keep in mind that the time
> step is 1ms, so the real time taken to solve 1 time step should be as
> close to 1ms as possible. In light of that, switching to calling the
> individual methods will make us 10 times slower than we should already
> be.
> Even if we change the time step to 10ms, that means only calling the
> individual methods is already chewing up all our time, not counting
> the linear system resolution.
> * [Paul] 2) What do you call not noticeable ?
> * [Paul] 3) If switching from a diagonal matrix to a dense matrix has
> no effect in time, I would argue we should drop Eigen for something
> else ! I suspect, from your 2 comments, that we are doing the wrong
> thing somewhere...and we should look into it.
Quickly, what I meant by not noticeable is that it had little effect on
> * [Paul] 1) If I understood properly, you see a 10ms time difference
> between calling addFMDK and calling the individual methods. That is
> actually very big in physics simulation. Keep in mind that the time
> step is 1ms, so the real time taken to solve 1 time step should be as
> close to 1ms as possible. In light of that, switching to calling the
> individual methods will make us 10 times slower than we should already
> be.
> Even if we change the time step to 10ms, that means only calling the
> individual methods is already chewing up all our time, not counting
> the linear system resolution.
> * [Paul] 2) What do you call not noticeable ?
> * [Paul] 3) If switching from a diagonal matrix to a dense matrix has
> no effect in time, I would argue we should drop Eigen for something
> else ! I suspect, from your 2 comments, that we are doing the wrong
> thing somewhere...and we should look into it.
the total performance. The variability on how long the physics took was
+-10ms. With and without my changes. So, I was saying the time was
identical as far as I could tell. But I was pointing out I wouldn't
notice changes less than 10ms, actually probably 5ms. Right now, the FEM
takes seconds, so no where near 1ms.
And in fact, if you comment out the inverting of the system matrix
OdeSolverEulerImplicit-inl.h:61, the update goes from taking 1.6s to
0.042s. So, of course I understand the importance of performance, but I
am merely trying to point out that we have focused our current design on
speeding up things that have an insignificant effect on the total
performance. At the cost of added complexity.
-Paul
Reply all
Reply to author
Forward
0 new messages