-
Notifications
You must be signed in to change notification settings - Fork 213
Correctly identify external Block boundaries for Topologies other than I1 #6521
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
Conversation
| external_boundaries_.emplace(direction); | ||
| if (has_boundary_in_this_direction) { | ||
| external_boundaries_.emplace(direction); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also assert here that the number of neighbors is 1 for cubes and either 1 or 6 for shells
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, that is not true in general (e.g. in a modified CylindricalCompactBinaryObject domain)
src/Domain/Structure/HasBoundary.hpp
Outdated
| /// \endcond | ||
|
|
||
| namespace domain { | ||
| /// \brief Whether or not a Topology has a boundary on a given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... side
| void test_spherical_shell() { | ||
| const Block<3> spherical_shell( | ||
| nullptr, 4, DirectionMap<3, BlockNeighbors<3>>{}, "Shell", | ||
| std::array{domain::Topology::I1, domain::Topology::S2Colatitude, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it help to define this array in Topology.hpp as spherical_shell_topology? It will come up more often
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly, though I'm not sure that is the right place to put it. We can discuss it at the next tech call.
Whether or not a boundary exists in a given direction depends on the Topology. Also add an ASSERT to check if an input neighbor is in a direction with a boundary.
Proposed changes
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments