From c9376637410fe56e46dca1626bd1384ba9e81f0a Mon Sep 17 00:00:00 2001 From: Susi Lehtola Date: Sat, 29 Aug 2026 18:31:33 +0300 Subject: [PATCH] Make the batcher and partition iterators usable SphericalMicroBatcher::const_iterator declared its comparison operators taking the non-const `iterator` type: bool operator==( iterator other ){ ... } There is no conversion between the two, so comparing two const_iterators was a hard error and the loop the class exists to support could not be written over a const batcher. A const batcher also had no begin()/end() at all, only cbegin()/cend(), so range-for over one failed by a second independent route. RadialGridPartition::rgp_iterator had the same class of bug in its post-increment, which declares `iterator retval = *this;` -- the enclosing class's non-const typedef, so const_iterator::operator++(int) could never instantiate. Fix both, and give the batcher const begin()/end(). While here, three related defects in the same iterators: * operator+ mutated *this and returned a reference to it. That is operator+= under the wrong name; at() worked only because it discards the iterator afterwards. Return a copy and take a difference_type. * difference_type was spelled `different_type` and iterator_category was spelled `iterator_catagory`, so std::iterator_traits saw neither and no standard algorithm could use these iterators. * With the traits now visible, `reference = value_type&` would have been a trap: operator* returns a prvalue, so binding a reference to it dangles. Use the proxy-iterator spelling instead (reference = value_type, pointer = void). range() and operator* are now const, as they never mutated anything. The existing test called cbatcher.at(i) but never iterated a const batcher; add both a range-for and an explicit cbegin()/cend() loop. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FDTFYJMQ76iujDFNHzZyXF --- .../batch/spherical_micro_batcher.hpp | 53 ++++++++++--------- .../pruned_spherical_quadrature.hpp | 23 ++++---- test/composite_quadratures.cxx | 20 +++++++ 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/include/integratorxx/batch/spherical_micro_batcher.hpp b/include/integratorxx/batch/spherical_micro_batcher.hpp index 03c848b..af905ea 100644 --- a/include/integratorxx/batch/spherical_micro_batcher.hpp +++ b/include/integratorxx/batch/spherical_micro_batcher.hpp @@ -255,10 +255,10 @@ class SphericalMicroBatcher { using value_type = std::tuple; - using different_type = size_t; - using pointer = value_type*; - using reference = value_type&; - using iterator_catagory = std::input_iterator_tag; + using difference_type = std::ptrdiff_t; + using pointer = void; + using reference = value_type; + using iterator_category = std::input_iterator_tag; index_iterator idx_it; point_iterator point_begin; @@ -275,17 +275,18 @@ class SphericalMicroBatcher { return retval; } - iterator& operator+(int i) { - idx_it += i; - return *this; - } + iterator operator+(difference_type i) const { + iterator copy = *this; + copy.idx_it += i; + return copy; + } - bool operator==( iterator other ){ return idx_it == other.idx_it; } - bool operator!=( iterator other ){ return !(*this == other); } + bool operator==( const iterator& other ) const { return idx_it == other.idx_it; } + bool operator!=( const iterator& other ) const { return !(*this == other); } - auto range() { + auto range() const { const auto idx = *idx_it; const auto idx_next = *(idx_it+1); @@ -302,7 +303,7 @@ class SphericalMicroBatcher { } - value_type operator*() { + value_type operator*() const { auto [npts,pb,pe,wb,we] = range(); auto [box_lo, box_up] = detail::get_box_bounds_points(pb, pe); @@ -325,10 +326,10 @@ class SphericalMicroBatcher { using value_type = std::tuple; - using different_type = size_t; - using pointer = value_type*; - using reference = value_type&; - using iterator_catagory = std::input_iterator_tag; + using difference_type = std::ptrdiff_t; + using pointer = void; + using reference = value_type; + using iterator_category = std::input_iterator_tag; const_index_iterator idx_it; const_point_iterator point_begin; @@ -346,17 +347,18 @@ class SphericalMicroBatcher { return retval; } - const_iterator& operator+(int i) { - idx_it += i; - return *this; - } + const_iterator operator+(difference_type i) const { + const_iterator copy = *this; + copy.idx_it += i; + return copy; + } - bool operator==( iterator other ){ return idx_it == other.idx_it; } - bool operator!=( iterator other ){ return !(*this == other); } + bool operator==( const const_iterator& other ) const { return idx_it == other.idx_it; } + bool operator!=( const const_iterator& other ) const { return !(*this == other); } - auto range() { + auto range() const { const auto idx = *idx_it; const auto idx_next = *(idx_it+1); @@ -373,7 +375,7 @@ class SphericalMicroBatcher { } - value_type operator*() { + value_type operator*() const { auto [npts,pb,pe,wb,we] = range(); auto [box_lo, box_up] = detail::get_box_bounds_points(pb, pe); @@ -465,6 +467,9 @@ class SphericalMicroBatcher { quad_->weights().cbegin() ); } + const_iterator begin() const { return cbegin(); } + const_iterator end() const { return cend(); } + typename iterator::value_type at( size_t i ) { if( i >= nbatches() ) throw std::runtime_error("Index out of bounds"); diff --git a/include/integratorxx/composite_quadratures/pruned_spherical_quadrature.hpp b/include/integratorxx/composite_quadratures/pruned_spherical_quadrature.hpp index d90305f..9d2f5e6 100644 --- a/include/integratorxx/composite_quadratures/pruned_spherical_quadrature.hpp +++ b/include/integratorxx/composite_quadratures/pruned_spherical_quadrature.hpp @@ -84,10 +84,10 @@ class RadialGridPartition { using index_range_type = std::pair; using quad_type = Quadrature; using value_type = std::pair; - using difference_type = size_t; - using pointer = value_type*; - using reference = value_type&; - using iterator_catagory = std::input_iterator_tag; + using difference_type = std::ptrdiff_t; + using pointer = void; + using reference = value_type; + using iterator_category = std::input_iterator_tag; index_iterator idx_it; quad_iterator quad_it; @@ -98,21 +98,22 @@ class RadialGridPartition { rgp_iterator& operator++(){ idx_it++; quad_it++; return *this; } rgp_iterator operator++(int) { - iterator retval = *this; + rgp_iterator retval = *this; ++(*this); return retval; } - rgp_iterator& operator+(int i) { - idx_it += i; - quad_it += i; - return (*this); + rgp_iterator operator+(difference_type i) const { + rgp_iterator copy = *this; + copy.idx_it += i; + copy.quad_it += i; + return copy; } - bool operator==(rgp_iterator other) const { + bool operator==(const rgp_iterator& other) const { return idx_it == other.idx_it && quad_it == other.quad_it; } - bool operator!=(rgp_iterator other) const { return !(*this == other); } + bool operator!=(const rgp_iterator& other) const { return !(*this == other); } value_type operator*() { return std::make_pair( std::make_pair(*idx_it, *(idx_it+1)), *quad_it ); diff --git a/test/composite_quadratures.cxx b/test/composite_quadratures.cxx index 435cd8a..02fdf79 100644 --- a/test/composite_quadratures.cxx +++ b/test/composite_quadratures.cxx @@ -155,6 +155,26 @@ TEST_CASE( "Spherical Quadratures", "[sph-quad]" ) { CHECK( npts_c == npts ); + // Iterating a const batcher: const_iterator compared against the + // non-const iterator type, so this could not previously compile. + npts_c = 0; + for( auto&& [box_lo, box_up, points_b, weights_b] : cbatcher ) { + + auto npts_b = points_b.size(); + CHECK( npts_b != 0 ); + npts_c += npts_b; + + } + + CHECK( npts_c == npts ); + + npts_c = 0; + for( auto it = cbatcher.cbegin(); it != cbatcher.cend(); ++it ) { + npts_c += std::get<2>(*it).size(); + } + + CHECK( npts_c == npts ); + auto batcher_clone = batcher.clone(); CHECK( &batcher.quadrature() != &batcher_clone.quadrature() ); CHECK( batcher.npts() == batcher_clone.npts() );