Skip to content
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

[SofaBaseMechanics] Fix UniformMass topology changes handling mode. #2377

Merged
merged 16 commits into from
Oct 28, 2021

Conversation

epernod
Copy link
Contributor

@epernod epernod commented Oct 1, 2021

This PR depends on #2374 #2375 and #2376 and should be rebased before review.

  • Remove old method handleTopologyChange
  • Turn d_indices into TopologySubsetIndices, this automatically fix the topological changes update.
  • Fix the use of subset indices combined with topology changes.
  • Add callback on endingEvent to update the totalMass/vertexMass
  • Fix the center of gravity display when using subset of indices

Here is an example of scenes RemovingTrianglesProcess.scn using a subset of indices (yellow points)
<UniformMass template="Vec3d,double" name="mass" totalMass="1.0" handleTopologicalChanges="1" indices="0 1 2 3 4 5 6 7 8 9"/>

RemovingTrianglesProcess_00000001

RemovingTrianglesProcess_00000002



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

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

@epernod epernod added pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request labels Oct 1, 2021
@epernod epernod added this to the v21.12 milestone Oct 1, 2021
@epernod epernod self-assigned this Oct 1, 2021
@epernod
Copy link
Contributor Author

epernod commented Oct 4, 2021

[ci-build][with-all-tests]

@@ -65,8 +69,7 @@ class UniformMass : public core::behavior::Mass<DataTypes>
/// indices outside of this range are discarded (useful for parallelization
/// using mesh partitionning)
Data< type::Vec<2,int> > d_localRange;
Data< type::vector<int> > d_indices; ///< optional local DOF indices. Any computation involving only indices outside of this list are discarded

SetIndex d_indices; ///< optional local DOF indices. Any computation involving only indices outside of this list are discarded
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same SetIndex is actually quite confusing, it's hiding that it corresponds to a data..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok what name do you want?
This is the same typedef as in all constraints component. Like FixedConstraint

…formMass.inl

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
@epernod epernod added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Oct 13, 2021
@@ -65,8 +69,7 @@ class UniformMass : public core::behavior::Mass<DataTypes>
/// indices outside of this range are discarded (useful for parallelization
/// using mesh partitionning)
Data< type::Vec<2,int> > d_localRange;
Data< type::vector<int> > d_indices; ///< optional local DOF indices. Any computation involving only indices outside of this list are discarded

SetIndex d_indices; ///< optional local DOF indices. Any computation involving only indices outside of this list are discarded
Data<bool> d_handleTopologicalChanges; ///< The mass and totalMass are recomputed on particles add/remove.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add the info specifying why this option exists (UniformMass works also only with DOFs - without TopologyContainer in the scene)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New proposal: remove the data, and keep it as a private variable of the UniformMass class.
This boolean variable should be true if a Container is found, false else.

epernod and others added 4 commits October 21, 2021 00:08
…formMass.inl

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
…otalMass, massDensity to send warnings only if values are strictly negatives
…default if the topology is dynamic register callback on DataSetIndex
@epernod epernod removed the pr: status wip Development in the pull-request is still in progress label Oct 25, 2021
@epernod epernod added the pr: status to review To notify reviewers to review this pull-request label Oct 25, 2021
@epernod
Copy link
Contributor Author

epernod commented Oct 25, 2021

[ci-build][with-all-tests]

{
msg_info() << "link to Topology container should be set to ensure right behavior. First Topology found in current context will be used.";
l_topology.set(this->getContext()->getMeshTopologyLink());
Real newVertexMass = d_totalMass.getValue() / Real(newSize);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if newSize == 0

@hugtalbot
Copy link
Contributor

Just tested it @epernod it works as a charm 👍 👍 👍 👍

@epernod epernod added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Oct 27, 2021
@epernod epernod added pr: status to review To notify reviewers to review this pull-request and removed pr: status ready Approved a pull-request, ready to be squashed labels Oct 27, 2021
@fredroy fredroy added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Oct 28, 2021
@guparan guparan merged commit 6068934 into sofa-framework:master Oct 28, 2021
@epernod epernod deleted the inf_uniformMass branch October 31, 2021 14:39
fredroy added a commit to fredroy/sofa that referenced this pull request Nov 8, 2021
…ofa-framework#2377)

This PR depends on sofa-framework#2374 sofa-framework#2375 and sofa-framework#2376 and should be rebased before review.

- Remove old method `handleTopologyChange`
- Turn d_indices into TopologySubsetIndices, this automatically fix the topological changes update. 
- Fix the use of subset indices combined with topology changes.
- Add callback on endingEvent to update the totalMass/vertexMass
- Fix the center of gravity display when using subset of indices

______________________________________________________

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix Fix a bug pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants