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

[Mass] Replace doUpdateInternal by callback: UniformMass #3927

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

hugtalbot
Copy link
Contributor

@hugtalbot hugtalbot commented Jun 7, 2023

In the spirit of #3900 and following #3924, this PR applies the change on the UniformMass.

To be noted, the componentState must be added to trigger the callback (done in addMDx(), addMToMatrix, accFromF and buildMassMatrix)

[ci-depends-on https://github.com/SofaDefrost/SoftRobots/pull/272]
[ci-depends-on https://github.com/sofa-framework/Registration/pull/14]


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).

@hugtalbot hugtalbot added pr: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels Jun 7, 2023
@hugtalbot
Copy link
Contributor Author

only the MeshMatrixMass to go @damienmarchal and .. soon goodbye doUpdateInternal !

@hugtalbot
Copy link
Contributor Author

trackInternalData to remove

@fredroy
Copy link
Contributor

fredroy commented Jun 13, 2023

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

@hugtalbot hugtalbot 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 Jun 28, 2023
@alxbilger alxbilger force-pushed the 202306_empty_doUpdateInternal_UniformMass branch from e99cf8c to c0cb154 Compare March 29, 2024 08:35
@alxbilger alxbilger added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Mar 29, 2024
@alxbilger
Copy link
Contributor

only the MeshMatrixMass to go @damienmarchal and .. soon goodbye doUpdateInternal !

I like your definition of "soon" 😆

@alxbilger
Copy link
Contributor

Is this->trackInternalData(d_vertexMass); still necessary? @hugtalbot

@fredroy fredroy force-pushed the 202306_empty_doUpdateInternal_UniformMass branch from e87252a to 5b5c29e Compare April 1, 2024 23:41
@hugtalbot
Copy link
Contributor Author

Similar feature is merged for the ConstantForceField in #3924. It adds separate callbacks for each data but a hacky flag allows to avoid data cross-dependency by de-activating all inputs non-set at the initialization. It could be done here as well but a more robust implementation (Data, callback) could be found.

Add a unit test to make sure that no data cross-dependency is triggered. Manual test as well.

@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress pr: status to review To notify reviewers to review this pull-request and removed pr: status to review To notify reviewers to review this pull-request pr: status wip Development in the pull-request is still in progress labels Apr 3, 2024
@hugtalbot
Copy link
Contributor Author

it seems from the CI that many tests and scene examples are using UniformMass without any input mass info ..
🤡 👺

@hugtalbot
Copy link
Contributor Author

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

@sofabot
Copy link
Collaborator

sofabot commented Apr 10, 2024

[ci-depends-on] detected during build #16.

To unlock the merge button, you must

@fredroy fredroy force-pushed the 202306_empty_doUpdateInternal_UniformMass branch from 2ce47ea to 14a5ee9 Compare April 15, 2024 00:26
@sofabot
Copy link
Collaborator

sofabot commented Apr 15, 2024

[ci-depends-on] detected during build #17.

To unlock the merge button, you must

Comment on lines +275 to +276
if(!this->isComponentStateValid())
msg_error() << "Initialization process is invalid";
Copy link
Contributor Author

@hugtalbot hugtalbot Apr 17, 2024

Choose a reason for hiding this comment

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

Review was taken into account last week

@bakpaul bakpaul 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 Apr 17, 2024
@fredroy fredroy force-pushed the 202306_empty_doUpdateInternal_UniformMass branch from 14a5ee9 to a2f9e25 Compare April 17, 2024 23:57
@sofabot
Copy link
Collaborator

sofabot commented Apr 17, 2024

[ci-depends-on] detected during build #18.

To unlock the merge button, you must

@fredroy fredroy force-pushed the 202306_empty_doUpdateInternal_UniformMass branch from a2f9e25 to b20193f Compare April 22, 2024 04:54
@sofabot
Copy link
Collaborator

sofabot commented Apr 22, 2024

[ci-depends-on] detected during build #19.

All dependencies are merged/closed and all ExternalProject pointers are up-to-date. Congrats! 👍

@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status ready Approved a pull-request, ready to be squashed labels Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: clean Cleaning the code pr: status wip Development in the pull-request is still in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants