-
Notifications
You must be signed in to change notification settings - Fork 9
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
refactor point_properties and property_container #509
base: issue-493
Are you sure you want to change the base?
Conversation
It seems that computing the derived properties on the fly instead of storing them has no performance impact, probably we should use this implementation instead.
|
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.
Minor changes
using Base::Base; | ||
constexpr static int _counter = __COUNTER__; | ||
|
||
DEFINE_CONTAINER(rho_inverse) |
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.
This is awesome!
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.
Ok, this is nuts.
: public impl::impl_properties_container< | ||
specfem::element::medium_tag::acoustic, | ||
specfem::element::property_tag::isotropic, 2> { | ||
using Base = |
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.
can we use base_type
here.
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.
updated
template <specfem::element::medium_tag MediumTag, | ||
specfem::element::property_tag PropertyTag, int N> | ||
struct impl_properties_container { | ||
using view_type = typename Kokkos::View<type_real ****, Kokkos::LayoutLeft, |
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 view type shoudl be Kokkos::View<type_real ***[N], Kokkos::LayoutLeft, ...>
since we know the value of N at compile time.
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.
Beautiful
DEFINE_PROP(c12) ///< @f$ c_{12} @f$ | ||
DEFINE_PROP(c23) ///< @f$ c_{23} @f$ | ||
DEFINE_PROP(c25) ///< @f$ c_{25} @f$ | ||
DEFINE_PROP(rho) ///< Density @f$ \rho @f$ |
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.
So Counter works, because at each first property use you store its value and subtract it. Neat.
using Base::Base; | ||
constexpr static int _counter = __COUNTER__; | ||
|
||
DEFINE_CONTAINER(rho_inverse) |
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.
Ok, this is nuts.
Description
point_property.mu()
rho_vp
) are now computed on the flyIssue Number
Closes #489
Checklist
Please make sure to check developer documentation on specfem docs.