Skip to content

DRAFT: Changes/fixes for adjset generation. #1434

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

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

BradWhitlock
Copy link
Member

@BradWhitlock BradWhitlock commented May 3, 2025

I've been running into a problem with adjset generation where points on either side of a domain will have scrambled point index orderings in the adjsets. If one uses the adjsets to index into the coordsets, it is possible to get points that do not match on either side of a domain interface. I figured out this happens because of the distance and tolerance comparison in ffloat::operator<. Apparently, we get points that are closer than CONDUIT_EPSILON (1.e-12) and the less than comparison fails. This affects PointTuple::operator< and ultimately the index at which new entities are inserted into the entity vector used to build the adjset.

If the epsilon is decreased then the adjset is fixed for the dataset that causes the problem. This does not seem like a robust enough fix so I'm still thinking about solutions. We should discuss.

Other changes:

  • Add some using statements for type readability
  • Use accessors to populate the adjset data

@BradWhitlock BradWhitlock marked this pull request as draft May 3, 2025 01:44
@BradWhitlock
Copy link
Member Author

I implemented adjset point sorting based on spatial bins. This works for the Blueprint tests but for some reason, it totally fails on the test problem I've been debugging.

I have some luck with a custom predicate passed to std::upper_bound() that checks whether points are the same within CONDUIT_EPSILON before delegating to PointTuple::operator<. That works. Is it better/robust? Meh.

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.

1 participant