-
Notifications
You must be signed in to change notification settings - Fork 8
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
Template lambda for scalar calls #363
Conversation
This is now ready for review. @dholladay00 @gopsub @jhp-lanl |
Ping @jhp-lanl @dholladay00 reminder to review. |
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.
Looks good to me. I appreciate the is_nullptr
function. I think it's very clear now what's going on and if I want to wade into the template nonsense I can look at the variadic_utils.hpp
file 🙂
@dholladay00 I think I addressed all your comments. Can you re-review and approve if you're happy? |
Is this passing gitlab CI @Yurlungur ? |
Running. Won't merge until it passes. |
Addressed @mauneyc-LANL 's comments. As soon as tests pass on re-git I will merge. |
Commenting here for posterity that when we switch to C++17 we will be able to do the following, which will allow us to properly handle passing in template <typename T>
struct remove_cvref {
typedef std::remove_cv_t<std::remove_reference_t<T>> type;
};
template <typename T>
using remove_cvref_t = typename remove_cvref<T>::type;
template<typename T>
constexpr inline bool is_null(T &&t) {
if constexpr(std::is_pointer<remove_cvref_t<T>>::value) {
return std::forward<T>(t) == nullptr;
} else {
return false;
}
}
#define SG_DO_IF_NOT_NULL(lambda) \
if constexpr (!std::is_null_pointer<remove_cvref_t<decltype(lambda)>>::value) \
if (!is_null(lambda))
template<typename T>
void DoSomething(T &&t) {
SG_DO_IF_NOT_NULL(t) {
std:: cout << t[0] << std::endl;
}
}
int main() {
auto q = nullptr;
std::vector<Real> v = {1, 2, 3, 4};
DoSomething(v);
DoSomething(q);
return 0;
} |
Tests pass on re-git. Merging. |
PR Summary
The work @gopsub is doing requires passing in many mesh-sized arrays into the lambda parameter. Currently on a per-cell basis, this requires packing those arrays into local contiguous arrays for lambdas. This can be cumbersome and expensive, especially for EOSs where that data is unneeded and is just a no-op.
This PR adds indexers/accessors to the lambda parameter for scalar calls, meaning data can now be passed in in a layout agnostic way. This should also reduce pain points in phoebus/riot, where the lambda arrays are used regularly and currently require contiguous accesses.
PR Checklist
make format
command after configuring withcmake
.If preparing for a new release, in addition please check the following: