Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions include/integratorxx/quadratures/s2/ahrens_beylkin.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <integratorxx/quadrature.hpp>
#include <integratorxx/quadratures/s2/ahrens_beylkin/ahrens_beylkin_grids.hpp>
#include <integratorxx/util/copy_grid.hpp>
#include <integratorxx/util/unsupported_grid.hpp>
#include <vector>
#include <cmath>

Expand Down Expand Up @@ -65,8 +66,6 @@ template <typename RealType> struct quadrature_traits<AhrensBeylkin<RealType>> {
detail::copy_grid<ahrens_beylkin_432<RealType>>(points, weights);
else if (npts == 492)
detail::copy_grid<ahrens_beylkin_492<RealType>>(points, weights);
else if (npts == 552)
detail::copy_grid<ahrens_beylkin_552<RealType>>(points, weights);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the dispatch gone, ahrens_beylkin_552.hpp can't be reached, but it's still #included from ahrens_beylkin_grids.hpp:12, so every TU that includes the Ahrens-Beylkin grids still parses its 1,122 lines. Could we delete the file and that include here? Git keeps the data, and #113 brings order 39 back as 572.

else if (npts == 612)
detail::copy_grid<ahrens_beylkin_612<RealType>>(points, weights);
else if (npts == 672)
Expand Down Expand Up @@ -163,6 +162,8 @@ template <typename RealType> struct quadrature_traits<AhrensBeylkin<RealType>> {
detail::copy_grid<ahrens_beylkin_7512<RealType>>(points, weights);
else if (npts == 15012)
detail::copy_grid<ahrens_beylkin_15012<RealType>>(points, weights);
else
detail::throw_unsupported_grid_size("AhrensBeylkin", npts);

return std::make_tuple(points, weights);
}
Expand All @@ -183,8 +184,6 @@ inline static int64_t npts_by_algebraic_order(int64_t order) {
return 432;
case 37:
return 492;
case 39:
return 552;
case 41:
return 612;
case 44:
Expand Down Expand Up @@ -302,8 +301,6 @@ inline static int64_t algebraic_order_by_npts(int64_t npts) {
return 35;
case 492:
return 37;
case 552:
return 39;
case 612:
return 41;
case 672:
Expand Down Expand Up @@ -421,8 +418,6 @@ inline static int64_t next_algebraic_order(int64_t order) {
return 35;
else if (order <= 37)
return 37;
else if (order <= 39)
return 39;
else if (order <= 41)
return 41;
else if (order <= 44)
Expand Down
3 changes: 3 additions & 0 deletions include/integratorxx/quadratures/s2/delley.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <integratorxx/quadrature.hpp>
#include <integratorxx/util/copy_grid.hpp>
#include <integratorxx/util/unsupported_grid.hpp>
#include <integratorxx/quadratures/s2/delley/delley_grids.hpp>
#include <vector>
#include <cmath>
Expand Down Expand Up @@ -86,6 +87,8 @@ struct quadrature_traits<Delley<RealType>> {
detail::copy_grid<delley_3074<RealType>>(points, weights);
else if(npts == 3470)
detail::copy_grid<delley_3470<RealType>>(points, weights);
else
detail::throw_unsupported_grid_size("Delley", npts);

// Pretabulated weights are missing 4 pi
for(auto i=0; i < npts; i++)
Expand Down
8 changes: 8 additions & 0 deletions include/integratorxx/quadratures/s2/lebedev_laikov.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <integratorxx/quadrature.hpp>
#include <integratorxx/quadratures/s2/lebedev_laikov/lebedev_laikov_grids.hpp>
#include <integratorxx/util/copy_grid.hpp>
#include <integratorxx/util/unsupported_grid.hpp>
#include <vector>
#include <cmath>

Expand Down Expand Up @@ -106,8 +107,12 @@ struct quadrature_traits< LebedevLaikov<RealType> > {
detail::copy_grid<lebedev_laikov_4334<RealType>>( points, weights );
else if( npts == 4802 )
detail::copy_grid<lebedev_laikov_4802<RealType>>( points, weights );
else if( npts == 5294 )
detail::copy_grid<lebedev_laikov_5294<RealType>>( points, weights );
else if( npts == 5810 )
detail::copy_grid<lebedev_laikov_5810<RealType>>( points, weights );
else
detail::throw_unsupported_grid_size("LebedevLaikov", npts);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One input still escapes this: size_t(-1), which is what npts_by_algebraic_order's -1 for an untabulated order becomes when it's passed straight to the constructor. The containers are sized from npts before the dispatch runs (lines 45–46, and the same in the other three families), so it fails with std::length_error from the vector constructor instead. Checking the size before allocating would make the runtime_error hold for every input, which #106's README now promises.


// Pretabulated weights are missing 4 pi
for(size_t i=0; i < npts; i++)
Expand Down Expand Up @@ -150,6 +155,7 @@ struct quadrature_traits< LebedevLaikov<RealType> > {
case 107: return 3890 ;
case 113: return 4334 ;
case 119: return 4802 ;
case 125: return 5294 ;
case 131: return 5810 ;
default: return -1;
}
Expand Down Expand Up @@ -189,6 +195,7 @@ struct quadrature_traits< LebedevLaikov<RealType> > {
case 3890: return 107 ;
case 4334: return 113 ;
case 4802: return 119 ;
case 5294: return 125 ;
case 5810: return 131 ;
default: return -1;
}
Expand Down Expand Up @@ -228,6 +235,7 @@ struct quadrature_traits< LebedevLaikov<RealType> > {
else if( order <= 107) return 107;
else if( order <= 113) return 113;
else if( order <= 119) return 119;
else if( order <= 125) return 125;
else return 131;

}
Expand Down
5 changes: 5 additions & 0 deletions include/integratorxx/quadratures/s2/womersley.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <integratorxx/quadrature.hpp>
#include <integratorxx/util/copy_grid.hpp>
#include <integratorxx/util/unsupported_grid.hpp>
#include <integratorxx/util/create_array.hpp>
#include <integratorxx/quadratures/s2/womersley/womersley_grids.hpp>
#include <vector>
Expand Down Expand Up @@ -185,6 +186,8 @@ struct quadrature_traits<Womersley<RealType>> {
detail::copy_grid<womersley_7814<RealType>>(points, weights);
else if(npts == 7939)
detail::copy_grid<womersley_7939<RealType>>(points, weights);
else
detail::throw_unsupported_grid_size("Womersley", npts);
}

inline static std::tuple<point_container, weight_container> generate(
Expand Down Expand Up @@ -320,6 +323,8 @@ struct quadrature_traits<Womersley<RealType>> {
detail::copy_grid<womersley_1923<RealType>>(points, weights);
else if(npts == 1986)
detail::copy_grid<womersley_1986<RealType>>(points, weights);
else
detail::throw_unsupported_grid_size("Womersley", npts);
return std::make_tuple(points, weights);
}

Expand Down
33 changes: 33 additions & 0 deletions include/integratorxx/util/unsupported_grid.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#pragma once

#include <cstddef>
#include <stdexcept>
#include <string>

namespace IntegratorXX {
namespace detail {

/**
* @brief Report a request for an angular grid size that is not tabulated.
*
* The angular quadratures are only defined for the specific point counts
* that integrate spherical harmonics exactly up to a given algebraic
* order. Requesting any other size is a programming error rather than a
* recoverable condition, but it is diagnosed at runtime because the size
* is typically read from user input.
*
* @param[in] family Name of the angular quadrature family
* @param[in] npts Unsupported number of points that was requested
*/
[[noreturn]] inline void throw_unsupported_grid_size(const char* family,
size_t npts) {
throw std::runtime_error(
std::string("IntegratorXX: ") + family + " does not tabulate a " +
std::to_string(npts) +
"-point grid. Supported sizes are given by "
"quadrature_traits<" +
family + ">::npts_by_algebraic_order(order).");
}

} // namespace detail
} // namespace IntegratorXX
53 changes: 53 additions & 0 deletions test/spherical_generator.cxx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "catch2/catch_all.hpp"
#include <iostream>
#include <algorithm>
#include <cmath>

#include <integratorxx/quadratures/radial.hpp>
#include <integratorxx/quadratures/s2.hpp>
Expand Down Expand Up @@ -290,6 +292,57 @@ TEMPLATE_LIST_TEST_CASE("S2 Generator", "[sph-gen]", s2_test_types) {

}

/// Exact value of \int_{S^2} x^{2a} y^{2b} z^{2c} d\Omega
static double exact_even_monomial(int a, int b, int c) {
auto dfact = [](int n) { double r = 1.0; for(int k = n; k > 1; k -= 2) r *= k; return r; };
return 4.0 * M_PI * dfact(2*a-1) * dfact(2*b-1) * dfact(2*c-1) /
dfact(2*(a+b+c) + 1);
}

TEMPLATE_LIST_TEST_CASE("S2 Grid Validity", "[sph-gen]", s2_test_types) {
using namespace IntegratorXX;
using angular_type = TestType;
using angular_traits = quadrature_traits<angular_type>;

// Sizes that no angular scheme tabulates must be diagnosed rather than
// silently yielding a zero grid.
REQUIRE_THROWS_AS( angular_type(1), std::runtime_error );
REQUIRE_THROWS_AS( angular_type(7), std::runtime_error );

// Every size advertised by the algebraic order tables must be dispatched
// and must integrate polynomials exactly up to its claimed order. A
// missing dispatch branch would return zero weights; a corrupt table
// would integrate low-order monomials incorrectly.
for( int64_t order = 0; order <= 200; ++order ) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop stops at order 200, but Ahrens-Beylkin advertises 15012 points at order 210, so that size is never dispatched or checked here. Could the bound cover it, e.g. order <= 210, or angular_traits::next_algebraic_order of some large order, which clamps to each family's maximum?

const auto npts = angular_traits::npts_by_algebraic_order(order);
if( npts < 0 ) continue;

INFO( "algebraic order " << order << ", " << npts << " points" );
angular_type aq(npts);
REQUIRE( aq.npts() == static_cast<size_t>(npts) );

const auto& p = aq.points();
const auto& w = aq.weights();

// Total degree 2*(a+b+c) must not exceed the claimed algebraic order.
// Capped for run time; degree 8 already exercises every table.
const int dmax = static_cast<int>(std::min<int64_t>(order / 2, 4));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says every advertised size must integrate exactly up to its claimed order, but dmax caps the check at degree 8, and that cap hides a second bad table. AB-5472 is advertised as order 127 but is exact only through l = 104. With the addition theorem, Σᵢ wᵢ P_l(n·xᵢ) = 4π δ_l0 at a few fixed unit vectors n, its relative residual reaches 3e-7 for l between 105 and 127. Every other advertised grid is exact through its claimed order to ≤ 6.1e-15, and at degree 8 5472 looks fine (1.1e-14), so the current test can't see it. As with 552, the data match Beylkin's qsph1-127-5472DP.dat bit for bit, so the defect is upstream.

Checking the full claimed order this way is cheap: the Legendre three-term recurrence per grid, O(N·L). All 230 advertised grids take about 3 s in NumPy, including parsing the headers. Could the test check every l up to the claimed order, with 5472 withdrawn (or regenerated) the same way as 552?

for( int a = 0; a <= dmax; ++a )
for( int b = 0; a + b <= dmax; ++b )
for( int c = 0; a + b + c <= dmax; ++c ) {
double integral = 0.0;
for( size_t i = 0; i < aq.npts(); ++i )
integral += w[i] * std::pow(p[i][0], 2*a)
* std::pow(p[i][1], 2*b)
* std::pow(p[i][2], 2*c);

INFO( "monomial x^" << 2*a << " y^" << 2*b << " z^" << 2*c );
REQUIRE_THAT( integral,
Catch::Matchers::WithinRel(exact_even_monomial(a,b,c), 1e-12) );
}
}
}

using sph_test_types = std::tuple<
std::tuple<bk_type, ah_type>,
std::tuple<bk_type, de_type>,
Expand Down
Loading