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

Add Morton numbers and expand LogicalLocation functionality #900

Merged
merged 72 commits into from
Jul 18, 2023

Conversation

lroberts36
Copy link
Collaborator

@lroberts36 lroberts36 commented Jun 28, 2023

PR Summary

In preparation for dealing with topological elements that are shared between blocks, this PR adds some utilities for calculating Morton numbers from logical locations and expands the functionality of LogicalLocation. In particular:

  1. A LogicalLocation instance now includes its Morton number.
  2. LogicalLocation member variables are private and are accessed through const accessors to prevent the logical location and Morton number from being inconsistent.
  3. >, <, and == operators are introduced for LogicalLocation since the locations have a Morton order.
  4. A custom hash function for LogicalLocation is provided so that we can make std::unordered_maps and std::sets with LogicalLocation as a key.
  5. Member functions for testing if a LogicalLocation contains, is contained in, or is a neighbor of another LogicalLocation are added.
  6. Member functions for getting parent and daughter LogicalLocations are added.
  7. Member functions for getting a set of possible LogicalLocations abutting a given topological element are added.
  8. Function for determining a blocks ownership of topological elements relative to a list of blocks.

A unit test that exercises this new functionality is included. The LogicalLocation of a neighbor is now stored in the NeighborBlock class.

PR Checklist

  • Code passes cpplint
  • New features are documented.
  • Adds tests for new features.
  • Code is formatted
  • Changes are summarized in CHANGELOG.md
  • CI has been triggered on Darwin for performance regression tests.
  • Docs build
  • (@lanl.gov employees) Update copyright on changed files

@Yurlungur
Copy link
Collaborator

planning to review this ASAP. Didn't get a chance today.

@lroberts36
Copy link
Collaborator Author

Thanks @Yurlungur. I thought of a couple things I could add to this PR tonight anyway.

Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

This is a nice addition and I can see it coming in handy both for code extensions and code cleanup going forward.

src/bvals/bvals_base.cpp Show resolved Hide resolved
src/utils/morton_number.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
src/defs.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@pdmullen pdmullen left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@pgrete pgrete left a comment

Choose a reason for hiding this comment

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

Nice work -- lots of bits flying around.
I only got a couple of clarification questions (though feel free to ignore and merge if you're on a tight schedule).
Similarly (assuming you can spare another 15 min or so), a (very brief) documentation would be great.
I remember that I had a similar question to the Athena++ devs in terms of "What's the best way to get a global index of a cell in an AMR sim?" it would be nice if we have that kind of info readily available in our docs.

tst/unit/test_logical_location.cpp Outdated Show resolved Hide resolved
tst/unit/test_logical_location.cpp Show resolved Hide resolved
src/mesh/logical_location.hpp Show resolved Hide resolved
src/mesh/logical_location.hpp Outdated Show resolved Hide resolved
src/mesh/logical_location.hpp Show resolved Hide resolved
@lroberts36
Copy link
Collaborator Author

@pgrete re-documentation: My plan was to write some comprehensive documentation as a separate PR. I know I say this all the time about updates and never do it, but I think it will be much easier to write a comprehensive document at one time than doing it piecemeal (and I swear I am not lying about eventually doing it).

@lroberts36 lroberts36 enabled auto-merge (squash) July 18, 2023 17:11
@lroberts36 lroberts36 merged commit badd6f4 into develop Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants