-
Notifications
You must be signed in to change notification settings - Fork 16
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
Value-View Interface #289
base: develop
Are you sure you want to change the base?
Value-View Interface #289
Conversation
…not be used in prod.
…nt. This might be useful again.
…; Moving SphArray.hh to Utilities/ManagedVector.hh
…e test for the new interface pattern.
…d organization for VVI macros; Differentiating between Ptr and Ref syntax metaclasses with vvi pattern.
…o provide a clearer API for using VVI to users.
… defines VIEW Ctor, Copy and Assign.
…Better default behavior for Value Interfaces, def copy-ctor, assign op and eq op behavior is baked in for free in the ValueInterface class.
…crtp; Cleaning up VVI_VALUE_ macros.
…or defining default Ctor, Copy, Ass and Eq operations.
…ngs in field_test
endif() | ||
message("-----------------------------------------------------------------------------") | ||
find_package(umpire REQUIRED NO_DEFAULT_PATH PATHS ${umpire_DIR}) | ||
if (umpire_FOUND) | ||
message("Found umpire External Package.") | ||
blt_convert_to_system_includes(TARGET umpire) |
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.
Should this blt_convert_to_system_includes() stuff also be in the block under "Found chai External Package" (and any other packages which could be found externally)?
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`ll add it to CHAI. We already do this for all other external packages on line 127.
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.
We don't do it for the packages we use find_package
on. We only do it on packages we manually bring in. Was this necessary for your stuff?
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.
It was, pedantic warnings from RAJA and umpire were causing our WARNINGS_AS_ERRORS builds to fail.
@@ -85,7 +90,10 @@ if(ENABLE_CUDA) | |||
#set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS} -arch=${CUDA_ARCH} --expt-relaxed-constexpr --extended-lambda -Xcudafe --display_error_number") | |||
set(CMAKE_CUDA_STANDARD 17) | |||
list(APPEND SPHERAL_CXX_DEPENDS cuda) | |||
set(SPHERAL_ENABLE_CUDA On) | |||
set(SPHERAL_ENABLE_VVI On) |
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.
Should the Value-View interface be enabled for CPU builds as well?
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.
It can be used for CPU builds but it is required for any GPU builds where we will be executing on the device. This just ensures it's enabled if you are building spheral w/ cuda.
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.
Looking good once tests all pass
Summary
ManagedVector
(MV
) implementation.MV
unit testing.VVI
) that is based around the experimentalchai::ManagedSharedPtr
.VVI
in Spheral RTD. Preview here.MV
member.MV
member.VVI
forSpheral::QuadraticInterpolator
.ctest
c++ unit testing for ensuring semantic behavior of classes is working as expected withVVI
.Spheral::QuadraticInterpolator
w/ and w/o VVI.Spheral::Field
. Incompletemake test
at the end of the build stage.SPHERAL_ENABLE_VVI=On
for+cuda
builds.ToDo :
RELEASE_NOTES.md
with notable changes.