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.

Sign up for GitHub

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

Jump to bottom

Refactor STEP3 #115

Merged
merged 12 commits into from
Jul 25, 2024
Merged

Refactor STEP3 #115

merged 12 commits into from
Jul 25, 2024

Conversation

damienmarchal
Copy link
Member

Worked on PointManager.
@adagolodjo is this a controller ? or no

@damienmarchal
Refactor PointManager
a79ee6f 
It looks like it is a controller... (but in that cas why exposing it to python ?)
Further refactoring should be added.
@damienmarchal
Cleaning
850eacb
@damienmarchal
Clean: LegendrePolynomialsMapping, fix licences
f0ac682
@damienmarchal
Remove un-needed BaseMapping
9d22b1e
@damienmarchal
Remove Un-needed includes.
2c7f194
@damienmarchal
Remove un-needed class alias declaration in public namespace
6ec68ef 
They are un-needed and bloat the namespace.
@adagolodjo
Copy link
Collaborator

Hi @damienmarchal, thanks for working on this component.
I will say that it is more of a topology manager than a controller. It is used to add or remove mechanical points in a topology. For instance, in the example of using the cosserat-based needle model, the component is used to create constraint points inside the volume. And these points are used to imitate the interaction of needle tissues.

@damienmarchal
Use type traites for template data types.
a4a530b
@damienmarchal
Remove useless includes in LegendrePolynomials.
0cd8be6
@damienmarchal
Remove un-needed #includes in the mappings
608aa7f 
It is important to always keep only the neededs includes.
Having un-needed includes:
	- slow down compilation time
	- increase (non linearly) the amount of file to recompile on change
	- make the code appears more complex and entangled that it is actually.
@damienmarchal
Use SReal and RGBA Color in RigidDistanceMapping data field.
2816a42
@damienmarchal
Clean data fields in DifferenceMultiMapping & RigidDistanceMapping
4e7713d
@damienmarchal
Remove the empty draw() method in some of the Cosserat&Legendre mapping.
30dc2dd
@damienmarchal
Copy link
Member Author

@adagolodjo did you check that the scenes are still working ?

On my side without unitttests it is very hard for me to be sure I'm not breaking things while refactoring. So I would advocate to really add much more automated testing an regression tests in cosserat

@adagolodjo
Copy link
Collaborator

Hi @damienmarchal,

Not yet, because I thought we would fix the problem on the CI first. But I'll take the branch and see if it compiles on my machine.
THANKS.

@adagolodjo adagolodjo merged commit 24a755b into pr-add-tests Jul 25, 2024
1 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adagolodjo adagolodjo adagolodjo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@damienmarchal @adagolodjo