Skip to content

Cleanup Constraints, added comments, docs, and a roadmap#2085

Merged
akenmorris merged 18 commits intomasterfrom
cleanup_constraints
Jul 15, 2023
Merged

Cleanup Constraints, added comments, docs, and a roadmap#2085
akenmorris merged 18 commits intomasterfrom
cleanup_constraints

Conversation

@HeavenlyBerserker
Copy link
Copy Markdown
Contributor

@HeavenlyBerserker HeavenlyBerserker commented Jun 1, 2023

Added a roadmap to navigate all the constraints code, additional comments in Doxygen-friendly format, a constraint group in the docs, and removed a ton of dead or useless code.

The only confusing parts are that we still have the constraint reads for both json and xml in the code, and the snap-to-surface routine still has the confusing name "ApplyConstraints".

Given the roadmap in "Libs/Optimize/Constraints/Constraints.cpp", I hope newer developers can edit and understand the constraints code better and can modify it more easily.

IMPORTANT: Completely removed SphereConstraints.

…rajectory calculations to apply constraints.
…. This was once projected to replace the snap to surface ApplyConstraints routine. In hindsight, it would have worked much better if we had used a lambda for each particles instead of each domain. However, the Newton update still yields more stable results so I'm removing it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Accidentally committed? We still need to fix this issue about test files being modified.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, should we just remove this file from tracking?

Copy link
Copy Markdown
Contributor

@akenmorris akenmorris Jul 14, 2023

Choose a reason for hiding this comment

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

Unfortunately, it's needed. Otherwise the fixed domain test fails.

@HeavenlyBerserker HeavenlyBerserker marked this pull request as draft June 13, 2023 21:22
@HeavenlyBerserker HeavenlyBerserker marked this pull request as ready for review July 11, 2023 18:13
Copy link
Copy Markdown
Contributor

@akenmorris akenmorris left a comment

Choose a reason for hiding this comment

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

Great!

@akenmorris akenmorris merged commit 9629116 into master Jul 15, 2023
@akenmorris akenmorris deleted the cleanup_constraints branch July 15, 2023 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants