-
Notifications
You must be signed in to change notification settings - Fork 32
Generalizes primal::evaluate_integral methods for componentwise integration
#1744
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
base: develop
Are you sure you want to change the base?
Conversation
kennyweiss
left a comment
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.
Thanks @jcs15c -- nice contribution!
| namespace internal | ||
| { | ||
| ///@{ | ||
| /// \name Boilerplate to support integrals of functions with general return types, |
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.
| /// \name Boilerplate to support integrals of functions with general return types, | |
| /// \name Type traits to support integrals of functions with general return types, |
| int_lb, | ||
| npts_Q, | ||
| npts_P); | ||
| total_integral += detail::evaluate_area_integral_component(beziers[j], |
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 like this needs to be updated:
| total_integral += detail::evaluate_area_integral_component(beziers[j], | |
| total_integral += detail::evaluate_area_integral_component(bez, |
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.
Thanks for the other changes!
| // Integrate the surface normal over the patches | ||
| ret_vec += evaluate_area_integral( | ||
| nPatch.getTrimmingCurves(), | ||
| [&nPatch](Point2D x) -> Vector<T, 3> { return nPatch.normal(x[0], x[1]); } npts); |
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.
Where did the comma in front of "npts" go? Isn't it still needed?
| ret_vec += evaluate_area_integral( | ||
| nPatch.getTrimmingCurves(), | ||
| [&nPatch](Point2D x) -> Vector<T, 3> { return nPatch.normal(x[0], x[1]); } npts); | ||
| }; |
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.
Extra semicolon. Delete it.
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 whole function call is a mess, apparently. Sorry about that, I'll fix it and double check the rest.
BradWhitlock
left a comment
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.
Thanks!
kennyweiss
left a comment
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.
Thanks for the updates @jcs15c -- looks great!
| EXPECT_NEAR(evaluate_scalar_line_integral(ellipse_arc, transc_integrand, npts), | ||
| 1.38837959326, | ||
| abs_tol); | ||
| EXPECT_NEAR(evaluate_line_integral(ellipse_arc, const_integrand, npts), 2.42211205514, abs_tol); |
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.
Please update the RELEASE-NOTES w/ the changed function name
…hub.com/LLNL/axom into feature/spainhour/general_eval_integral
| template <typename Lambda, | ||
| typename T, | ||
| int NDIMS, | ||
| typename LambdaRetTyp = std::invoke_result_t<Lambda, typename BezierCurve<T, NDIMS>::PointType>> |
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.
Throughout this file, sometimes I read LambdaRetType (as in (new) line 87 in the Doxygen), sometimes LambdaRetTyp (as here), and sometimes LambdaRetTYpe (as at (new) line 300). Please make them all consistent. If they have to be disambiguated, please make it a larger string or conceptual distance.
Arlie-Capps
left a comment
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.
Thanks, @jcs15c !
Summary
This PR is an enhancement, adding an additional
primal::evaluate_line_integralmethod toevaluate_integral.hppwhich permits integral evaluation over any lambda function which has an "integrable" return type, i.e. one for which addition and scalar multiplication are defined. Most prominently, this permits evaluation over integrands which returnprimal::Vectorobjects. The same functionality is added toprimal::evaluate_area_integral. These methods are supported by additional tests inprimal_integral.cpp.More generally, this addition is meant to simplify the process of evaluating physical quantities for geometric objects. For example, each component of the average surface normal of a 3D NURBS surface is computed by integrating each component of the surface normal vector over the surface. Evaluating this as three separate integrals results in a significant amount of redundant computation which can now be avoided.
A note on names
Implementing this feature necessarily introduces some amount of naming ambiguity between methods in
evaluate_integral.hpp, as now there are two types of "vector field integrals" with distinct meanings. Currently defined indevelopare methodsevaluate_vector_line_integral, which evaluates integrals of the formwhich assume that$\vec{F}$ : $\mathbb{R}^n \to \mathbb{R}^n$ . Typically $n$ is equal to 2 or 3, but technically other values are possible mathematically. Either way, this method always returns a scalar value, i.e. a double, as would be suggested by the dot product in the formulation.
The proposed changes introduces methods of evaluating integrals of the form
which assumes that$\vec{F}$ : $\mathbb{R}^n \to \mathbb{R}^m$ , with an output in $\mathbb{R}^m$ . While $n$ will most often be 2 or 3, $m$ can be significantly larger. Specifically, I'm targeting a future use with $m = 39$ , corresponding to a function with a return type $\mathbb{R}^n$ ) are themselves a "vector space over a scalar field" in a mathematical sense.
primal::Vector<double, 39>. These could also be characterized as "vector field integrals," even though they behave more like "component-wise scalar field integrals." This ambiguity is essentially unavoidable, because even the input for "scalar field integrals" (To make sure any ambiguity is inherently resolved by templates, the functionality of
evaluate_scalar_line_integralmethods inevaluate_integral.hppis merged into anevaluate_line_integralmethod which performs this more general component-wise integration. Conveniently, this mirrors theevaluate_area_integralmethod.