-
Notifications
You must be signed in to change notification settings - Fork 24
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
AMRtoLocal returns unsigned ints #779
AMRtoLocal returns unsigned ints #779
Conversation
WalkthroughWalkthroughThe recent changes involve updating the return types of functions Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- src/core/data/grid/gridlayout.hpp (2 hunks)
- tests/core/data/gridlayout/gridlayout_amr.cpp (1 hunks)
Additional comments: 2
tests/core/data/gridlayout/gridlayout_amr.cpp (1)
- 84-90: The change from
Box
toBox<std::uint32_t, 1>
reflects the update in theBox
class template parameters to enforce type safety and dimensionality. This is a good practice as it ensures that theBox
class is used with the correct type and dimension, which can prevent potential bugs and improve code readability. The test case has been updated accordingly to reflect these changes, which is necessary to maintain the consistency of the test suite with the updated codebase. However, ensure that all other usages ofBox
in the codebase are also updated to match this new template parameter requirement.src/core/data/grid/gridlayout.hpp (1)
- 836-842: The
AMRToLocal
function for converting aBox<T, dimension>
to aBox<std::uint32_t, dimension>
is correctly using the previously definedAMRToLocal
function forPoint<T, dimension>
. This ensures consistency in the conversion process. However, the same considerations about underflow and type safety from the previous comment apply here as well.
Concerns #558.
Cleaned up the previous commits and added the edit to gridlayout_amr following Philip's advice in #777.
Summary by CodeRabbit
Refactor
Tests