-
Notifications
You must be signed in to change notification settings - Fork 95
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 isFci also to mesh #3001
Add isFci also to mesh #3001
Conversation
Like #2887 for the mesh |
@ggeorgakoudis do you mind sharing the logs for the gitlab CI? |
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.
clang-tidy made some suggestions
clang-tidy review says "All clean, LGTM! 👍" |
Unfortunately the gitlab CI is behind the LLNL firewall so only LLNL employees have access 🙁. It's not ideal but at least it gives us a way to do GPU run tests. I suppose @bendudson and myself can paste logs as needed. |
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.
LGTM. Would be good to get a test on this.
std::shared_ptr<Coordinates> | ||
getCoordinatesConst(const CELL_LOC location = CELL_CENTRE) const { |
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.
I think ideally this would be:
std::shared_ptr<Coordinates> | |
getCoordinatesConst(const CELL_LOC location = CELL_CENTRE) const { | |
const Coordinates& | |
getCoordinates(const CELL_LOC location = CELL_CENTRE) const { |
but maybe this is too likely to cause issues elsewhere
@@ -828,6 +841,16 @@ public: | |||
ASSERT1(RegionID.has_value()); | |||
return region3D[RegionID.value()]; | |||
} | |||
bool isFci() const { |
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.
Please add a docstring
No description provided.