Skip to content

Conversation

Angelyr
Copy link
Collaborator

@Angelyr Angelyr commented Aug 6, 2025

Changes to coarsen

  • improvement to independent set
  • fixed bug that made it impossible to collapse edge in both directions
  • fixed bug that made it slower to collapse in one direction in parallel
  • added multi-step coarsen

Changes to snapping

  • fixed bug with snapping coarsening the mesh too mush
  • fixed bug where not all regions were included in first problem plane
  • should snap vertices in most cases
  • added quality settings for split collapse

More testing

  • added test for common errors in coarsen, refinement and snapping

Most of the these changes to snapping and coarsen came from Li's thesis with some minor exceptions:

  • In the coarsen procedure in Li's thesis it describes getting the shortest edge next to a vertex and collapsing it. We found we had better results if we try collapsing the shortest edge and then grab the next shortest until one succeeds.
  • Another difference is that Li's thesis mentions collapses, but it has very little detail in how those collapses are selected, so our logic for collapses is taken from our old mesh adapt library.
  • Li's thesis also describes using FaceSwap in snapping and vertex repositioning in snapping and coarsen, but we haven't created either of those operators yet.

@Angelyr Angelyr linked an issue Aug 27, 2025 that may be closed by this pull request
@Angelyr
Copy link
Collaborator Author

Angelyr commented Aug 30, 2025

@bobpaw Feel free to take a look at the code now

@cwsmith
Copy link
Contributor

cwsmith commented Sep 22, 2025

@bobpaw are you still reviewing?

@bobpaw
Copy link
Collaborator

bobpaw commented Sep 22, 2025

@bobpaw are you still reviewing?

Yes, sorry. I have a few more longer files to look at. I had also submitted a patch to Angel via email which adjusts his isCapstone function slightly. I'll finish up the review today.

Copy link
Collaborator

@bobpaw bobpaw left a comment

Choose a reason for hiding this comment

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

Sorry for lots of comments. After this PR is merged I will be working on adding Doxygen documentation for new and improved features.

Copy link
Collaborator

@bobpaw bobpaw left a comment

Choose a reason for hiding this comment

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

Looks good to me. Might be worth configuring with VALGRIND=valgrind and running ctests (probably excluding the long capstone tests -E '^capVol.*').

Copy link
Contributor

@cwsmith cwsmith left a comment

Choose a reason for hiding this comment

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

Looks good. I agree with @bobpaw about running valgrind/address sanitizer on the tests. A few small comments are below.

@cwsmith cwsmith removed the v4.1.0 changes included in the 4.1.0 release label Sep 29, 2025
@cwsmith
Copy link
Contributor

cwsmith commented Oct 2, 2025

@Angelyr The changes look good. Thank you. Once we have checked with valgrind/asan I think this will be ready to merge.

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.

ac/adapt-changes dpw6
4 participants