Compile-time GLL quadrature. by int-ptr-ptr · Pull Request #1891 · PrincetonUniversity/SPECFEMPP

int-ptr-ptr · 2026-06-01T20:25:47Z

Description

Please describe the changes/features in this pull request.

Issue Number

If there is an issue created for these changes, link it here

Checklist

Please make sure to check developer documentation on specfem docs.

I ran the code through pre-commit to check style
THE DOCUMENTATION BUILDS WITHOUT WARNINGS/ERRORS
I have added labels to the PR (see right hand side of the PR page)
My code passes all the integration tests
I have added sufficient unittests to test my changes
I have added/updated documentation for the changes I am proposing
I have updated CMakeLists to ensure my code builds
My code builds across all platforms

lsawade

Was working with compiler explorer to analyze what's happening and it found some things that could improve loading.

Compiler Explorer

lsawade · 2026-06-05T12:39:56Z

+ * @brief Populates poly.roots using the algo described in
+ * polynomial_zeros.ipynb


Those were my notes for rootfind when I initially wrote the code way back when.

lsawade · 2026-06-05T13:50:40Z

+
+  using numerical_type = Nodes::numerical_type;
+  static constexpr std::size_t N = Nodes::N;
+


Suggested change

private:

static constexpr auto coeff_table =

impl::make_lagrange_coeff_table(Nodes::get_nodes());

lsawade · 2026-06-05T13:57:52Z

+  struct Nodes {
+  private:
+    using initializer_node_type = decltype(NodeInitializer::get_nodes());
+


Suggested change

private:

static constexpr auto nodes = NodeInitializer::get_nodes();

constexpr does not guarantee compile-time evaluation. Since NodeInitializer could be anything, I purposefully wrapped NodeInitializer::get_nodes() with a consteval function in case consteval was forgotten.

I had set a private constexpr auto nodes = Nodes::get_nodes();, though, but I ended up removing that (elaborated in the comment about the nvcc issue below).

I re-added it since I found a bypass, so if this suggestion was based on a missing static constexpr variable, consider the suggestion followed. If this was a suggestion to remove the consteval Nodes::get_nodes(), I want to push back, due to the aforementioned behavior of constexpr.

lsawade · 2026-06-05T13:58:18Z

+     */
+    static constexpr KOKKOS_INLINE_FUNCTION numerical_type
+    node(const int &node_index) {
+      return get_nodes()[node_index];


Suggested change

return get_nodes()[node_index];

return nodes[node_index];

nvcc:

error: identifier "specfem::quadrature::compiletime::Lagrange< ::specfem::quadrature::compiletime::gll_initializer<(int)5, float> > ::Nodes::nodes" is undefined in device code

for some reason, declaring the constexpr nodes variable inside the KOKKOS_INLINE_FUNCTION does work, though.

This was the fix I ended up with:

static constexpr KOKKOS_INLINE_FUNCTION numerical_type node(const int &node_index) { constexpr auto nodes_ = nodes; // so for some reason this works, while directly referencing nodes has // nvcc crying return nodes_[node_index]; }

It's a little hacky, but my nvcc is okay with it.

lsawade · 2026-06-05T13:58:51Z

+    static consteval Kokkos::Array<numerical_type, N> get_nodes() {
+      return NodeInitializer::get_nodes();


Suggested change

static consteval Kokkos::Array<numerical_type, N> get_nodes() {

return NodeInitializer::get_nodes();

see above comment, quote-copied here for reference:

constexpr does not guarantee compile-time evaluation. Since NodeInitializer could be anything, I purposefully wrapped NodeInitializer::get_nodes() with a consteval function in case consteval was forgotten.

I'm taking the care to ensure consteval, since (to my understanding) the C++20 standard requires compile-time evaluation

consteval - specifies that a function is an immediate function, that is, every call to the function must produce a compile-time constant

while (from my conversations with Rohit) constexpr does not have such an explicit requirement.

lsawade · 2026-06-05T14:01:39Z

+    }
+    KOKKOS_INLINE_FUNCTION numerical_type
+    operator()(const int &node_index) const {
+      return node(node_index);


Suggested change

return node(node_index);

return nodes[node_index];

See comment on Nodes::node(const int&), quote-copied here for reference:

This was the fix I ended up with:

static constexpr KOKKOS_INLINE_FUNCTION numerical_type node(const int &node_index) { constexpr auto nodes_ = nodes; // so for some reason this works, while directly referencing nodes has // nvcc crying return nodes_[node_index]; }

It's a little hacky, but my nvcc is okay with it.

Rohit-Kakodkar

Can we just hard code the polynomials for degree 5 and 8? I think it would make this implementation a lot simpler and easier to maintain. Thoughts @lsawade

lsawade · 2026-06-09T13:39:18Z

While the legendre rootfinding is more elegant in the mathematical sense, I agree with @Rohit-Kakodkar that it would be much simpler to just take NGLL 5-12 and hardcode the GLL points instead for this. This is what I originally had commented as well: #1879

It would drastically reduce code complexity and these roots are not changing.

int-ptr-ptr added 7 commits June 1, 2026 16:24

squash rebase from compiletime-gll

2730b92

removed leftover source file reference in cmake

f11064c

fabs to constexpr from consteval

8380480

removed coeff var

d7fdb50

removed lambda from constexpr cauchy_bound

f671be5

changed tolerances for check, forgotten deep_copy

3765d29

changes for gpu compile

17188af

int-ptr-ptr requested review from Rohit-Kakodkar, icui and lsawade June 2, 2026 16:44

Merge branch 'devel' into compiletime-gll

ec8c61d

lsawade requested changes Jun 5, 2026

View reviewed changes

review changes 1

842a627

Rohit-Kakodkar reviewed Jun 9, 2026

View reviewed changes

removed rootfinding in favor of manual roots

ce6eebe

int-ptr-ptr requested review from Rohit-Kakodkar and lsawade July 14, 2026 15:47

		* @brief Populates poly.roots using the algo described in
		* polynomial_zeros.ipynb


		using numerical_type = Nodes::numerical_type;
		static constexpr std::size_t N = Nodes::N;

+private:
+  static constexpr auto coeff_table =
+      impl::make_lagrange_coeff_table(Nodes::get_nodes());



	private:
	static constexpr auto nodes = NodeInitializer::get_nodes();

		static consteval Kokkos::Array<numerical_type, N> get_nodes() {
		return NodeInitializer::get_nodes();

Uh oh!

Conversation

int-ptr-ptr commented Jun 1, 2026

Description

Issue Number

Checklist

Uh oh!

lsawade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

int-ptr-ptr Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rohit-Kakodkar left a comment

Choose a reason for hiding this comment

Uh oh!

lsawade commented Jun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

int-ptr-ptr Jun 5, 2026 •

edited

Loading