-
Notifications
You must be signed in to change notification settings - Fork 17
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
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
…ve undeifned symbols when theyre dynamically loaded.
mInterp(0.0, kernel.kernelExtent(), numPoints, Wlookup<KernelType>(kernel)), | ||
mGradInterp(0.0, kernel.kernelExtent(), numPoints, gradWlookup<KernelType>(kernel)), | ||
mGrad2Interp(0.0, kernel.kernelExtent(), numPoints, grad2Wlookup<KernelType>(kernel)), |
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.
@jmikeowen These are the changes I needed to revert. NVCC isn't happy with the local variable kernel in the lambda captures when compiling for the device.
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.
Ah I see, so you have to go back to externally defined functors. This is harmless enough, but kind of a bummer since I like the compact (and I think clearer) lambda function version. Do we think this is something that newer versions of nvcc can/will correct? Seems like they're not supporting standard C++ things here.
Also, since RAJA relies on lamda functions for it's trickery as well, what's teh salient difference here? Is it the reference capturing [&] version?
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.
RAJA requires capturing by value [=] so a copy of the object must be taken into a host-device lambda.
In this specific case the lambda is used for construction of the Interpreter. When VVI + Cuda are enabled the underlying ManagedSharedPtr tries to invoke the constructor on both the host and device. Meaning kernel
isn't available to the lambda invoked on the device, we need a copy of it to be passed over to the device in order to be able to call it correctly.
There are a couple of ways of getting back to an interface where we provide a lambda:
- Construct interpreters from factory functions that are able to construct w/ appropriate size on Host + Device, then fill the interpreter on the host and copy the contents to the device.
- Have the interprter take a handle to the kernel object then define the lambda as
[=](const double, const kernel) { return kernel ... }
so the interpreter can pass the kernel internally in it's constructor.
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.