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
15 changes: 10 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,17 @@ mp = MajoranaPropagator(operator, initial_state, cutoff=4)
2. Use C++23 syntax and idioms.
3. Use almost always auto style.
4. Use trailing return type syntax in function declarations.
5. Add a one-line `///` summary if the declaration is in `cpp/include/monoprop/`; elsewhere add a plain
5. Write a const/non-const accessor pair as one deducing-this member (`this Self &&self`, returning
`auto &`) instead of two bodies; reach for `std::forward_like<Self>` when the referent's const-ness
does not follow the owner's, as through a `unique_ptr`. Do **not** take the object parameter *by
value* to serve as an operator's working copy: that makes it a stack array, which loses NRVO and
trips `-fstack-protector-strong` — see the comment on `Bitset`'s bitwise operators.
6. Add a one-line `///` summary if the declaration is in `cpp/include/monoprop/`; elsewhere add a plain
`//` note only where the code does not already say it.
6. Implement in the corresponding `.cpp` under `cpp/monoprop/`.
7. Add Python bindings in `src/monoprop/bindings/binder.h`
8. Regenerate bindings with `tools/generate-binders.py`
9. Test with both C++ and Python tests
7. Implement in the corresponding `.cpp` under `cpp/monoprop/`.
8. Add Python bindings in `src/monoprop/bindings/binder.h`
9. Regenerate bindings with `tools/generate-binders.py`
10. Test with both C++ and Python tests

## Documentation Maintenance Policy

Expand Down
29 changes: 14 additions & 15 deletions cpp/include/monoprop/MPGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@
namespace monoprop {

/// Ordered per-rank record of the evolution circuit, one Layer per generator.
class monoprop_EXPORT MPGraph {
class monoprop_EXPORT MPGraph : public LayerWindow {
private:
using LayerIterator = std::vector<Layer>::iterator;
using ConstLayerIterator = std::vector<Layer>::const_iterator;

bool schrodinger_;
std::vector<Layer> layers_;
Expand All @@ -41,18 +40,17 @@

auto active_end_index() const -> size_t { return layers_.size(); }

auto active_begin_iterator() -> LayerIterator {
return layers_.begin() + static_cast<std::ptrdiff_t>(active_begin_index());
// Deducing this: const-ness of the returned iterator follows the object, so neither body is doubled.
template <typename Self>
auto active_begin_iterator(this Self &&self) {

Check failure on line 45 in cpp/include/monoprop/MPGraph.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

"std::forward" is never called on this forwarding reference argument.

See more on https://sonarcloud.io/project/issues?id=Algorithmiq_monoprop&issues=AaA_hChHCeBjfpzs_vZm&open=AaA_hChHCeBjfpzs_vZm&pullRequest=295
return self.layers_.begin() + static_cast<std::ptrdiff_t>(self.active_begin_index());
}

auto active_end_iterator() -> LayerIterator { return layers_.end(); }

auto active_begin_iterator() const -> ConstLayerIterator {
return layers_.begin() + static_cast<std::ptrdiff_t>(active_begin_index());
template <typename Self>
auto active_end_iterator(this Self &&self) {

Check failure on line 50 in cpp/include/monoprop/MPGraph.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

"std::forward" is never called on this forwarding reference argument.

See more on https://sonarcloud.io/project/issues?id=Algorithmiq_monoprop&issues=AaA_hChHCeBjfpzs_vZn&open=AaA_hChHCeBjfpzs_vZn&pullRequest=295
return self.layers_.end();
}

auto active_end_iterator() const -> ConstLayerIterator { return layers_.end(); }

auto append_position() -> LayerIterator { return schrodinger_ ? active_begin_iterator() : active_end_iterator(); }

auto append_layer(Layer layer) -> void { layers_.emplace(append_position(), std::move(layer)); }
Expand Down Expand Up @@ -91,11 +89,12 @@

auto layers() const -> size_t { return active_end_index() - active_begin_index(); }

auto get_layer(size_t layer_idx) -> Layer& { return layers_[checked_layer_offset(layer_idx)]; }

auto get_layer(size_t layer_idx) const -> const Layer& { return layers_[checked_layer_offset(layer_idx)]; }

auto get_layer_traversal(size_t layer_idx) const -> LayerTraversal { return get_layer(layer_idx).traversal(); }
/// The layer at `layer_idx` in build order; throws LayerIndexOutOfRange at or past the end.
// Deducing this: const-ness of the returned reference follows the object, so one body serves both.
template <typename Self>
auto get_layer(this Self &&self, size_t layer_idx) -> auto & {

Check failure on line 95 in cpp/include/monoprop/MPGraph.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

"std::forward" is never called on this forwarding reference argument.

See more on https://sonarcloud.io/project/issues?id=Algorithmiq_monoprop&issues=AaA_hChHCeBjfpzs_vZo&open=AaA_hChHCeBjfpzs_vZo&pullRequest=295
return self.layers_[self.checked_layer_offset(layer_idx)];
}

/// Non-owning replay view over the active layers, in build order.
auto replay_view() const -> MPGraphView { return {layers_, active_begin_index(), layers(), false}; }
Expand Down
26 changes: 12 additions & 14 deletions cpp/include/monoprop/MonomialPropagator.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,12 @@ class MonomialPropagator {
}

/// This rank's operator storage. Single-partition only — see require_single_partition_.
auto mp_op() -> detail::MPOperator<NumModes> & {
require_single_partition_("mp_op()");
return mp_op_;
}
auto mp_op() const -> const detail::MPOperator<NumModes> & {
require_single_partition_("mp_op()");
return mp_op_;
// Deducing this, so the partition guard is written once instead of once per const-ness. `Self` is left
// unconstrained: it deduces to a derived front-end when one calls this on itself, which is intended.
template <typename Self>
auto mp_op(this Self &&self) -> auto & {
self.require_single_partition_("mp_op()");
return self.mp_op_;
}

// The breakdown fields are additive over the disjoint hash partitions, so a facade sums them.
Expand Down Expand Up @@ -157,13 +156,12 @@ class MonomialPropagator {
auto set_parameter_mapping(const VecZ &parameter_mapping) -> void;

/// This rank's monomial → coefficient index. Single-partition only — see require_single_partition_.
auto indexing() -> detail::OperatorIndex<NumModes> & {
require_single_partition_("indexing()");
return *mp_op_.store;
}
auto indexing() const -> const detail::OperatorIndex<NumModes> & {
require_single_partition_("indexing()");
return *mp_op_.store;
template <typename Self>
auto indexing(this Self &&self) -> auto & {
self.require_single_partition_("indexing()");
// unique_ptr::operator* is const-qualified but yields a mutable referent, so the propagator's
// const-ness has to be re-applied by hand; plain `*store` would hand out a mutable index.
return std::forward_like<Self>(*self.mp_op_.store);
}

/// Per-layer (cos_inds, local_cycles, cross_rank_sin_send, cross_rank_sin_recv) for this
Expand Down
11 changes: 9 additions & 2 deletions cpp/monoprop/Bitset.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@
return *this;
}

// These deliberately keep a named local and a by-reference object parameter rather than taking
// `this Bitset self` by value: a by-value object parameter is a stack array, which loses NRVO and
// (under -fstack-protector-strong, the platform default) puts a frame and a canary check on the
// library's hottest primitive.
[[nodiscard]] constexpr auto operator~() const noexcept -> Bitset {
Bitset r = *this;
for (auto i = 0uz; i < kNumWords; ++i)
Expand Down Expand Up @@ -177,8 +181,11 @@
}

[[nodiscard]] static constexpr auto num_words() noexcept -> size_t { return kNumWords; }
[[nodiscard]] constexpr auto data() const noexcept -> const uint64_t * { return words_.data(); }
[[nodiscard]] constexpr auto data() noexcept -> uint64_t * { return words_.data(); }
// Deducing this: const-ness of the returned pointer follows the object.
template <typename Self>
[[nodiscard]] constexpr auto data(this Self &&self) noexcept -> auto * {

Check failure on line 186 in cpp/monoprop/Bitset.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

"std::forward" is never called on this forwarding reference argument.

See more on https://sonarcloud.io/project/issues?id=Algorithmiq_monoprop&issues=AaA_hCZjCeBjfpzs_vZl&open=AaA_hCZjCeBjfpzs_vZl&pullRequest=295
return self.words_.data();
}
[[nodiscard]] constexpr auto word(size_t i) const noexcept -> uint64_t { return words_[i]; }

[[nodiscard]] constexpr auto find_first() const noexcept -> size_t { // NumBits if none
Expand Down
14 changes: 11 additions & 3 deletions cpp/monoprop/detail/graph/MPGraphViews.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,19 @@ struct GraphMemoryBreakdown final {
}
};

// The layer-window vocabulary shared by MPGraph and its views: the deriving class supplies get_layer(),
// this supplies everything derivable from it. Deducing this rather than CRTP, so a deriving class need not
// name itself as a template argument, and the mixin stays an empty base.
struct LayerWindow {
template <typename Self>
auto get_layer_traversal(this const Self &self, size_t layer_idx) -> LayerTraversal {
return self.get_layer(layer_idx).traversal();
}
};

// `reverse` traverses the window newest-first (Schrödinger replay order). Non-owning — the layer vector
// must outlive the view.
class MPGraphView {
class MPGraphView : public LayerWindow {
public:
MPGraphView(const std::vector<Layer> &layers, size_t base, size_t count, bool reverse)
: layers_(&layers),
Expand All @@ -82,8 +92,6 @@ class MPGraphView {

auto get_layer(size_t layer_idx) const -> const Layer & { return (*layers_)[checked_layer_offset(layer_idx)]; }

auto get_layer_traversal(size_t layer_idx) const -> LayerTraversal { return get_layer(layer_idx).traversal(); }

private:
auto checked_layer_offset(size_t layer_idx) const -> size_t {
if (layer_idx >= count_) {
Expand Down
9 changes: 7 additions & 2 deletions cpp/monoprop/detail/partition/PartitionGroup.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <string>
#include <thread>
#include <type_traits>
#include <utility>
#include <vector>

#include "monoprop/detail/mpi/Comm.h"
Expand Down Expand Up @@ -106,8 +107,12 @@ class PartitionGroup {
~PartitionGroup() { stop_and_join_(); }

auto partition_count() const -> int { return n_; }
auto partition(int s) -> MonomialPropagator<NumModes> & { return *partitions_[static_cast<size_t>(s)]; }
auto partition(int s) const -> const MonomialPropagator<NumModes> & { return *partitions_[static_cast<size_t>(s)]; }
// Deducing this: unique_ptr::operator* yields a mutable referent whatever the owner's const-ness, so
// forward_like re-applies this group's.
template <typename Self>
auto partition(this Self &&self, int s) -> auto & {
return std::forward_like<Self>(*self.partitions_[static_cast<size_t>(s)]);
}

// Run `body(partition_rank)` on all masters, block until every one finishes, then rethrow the first
// exception raised (peers were released via poison, so a throw on one master never hangs the rest).
Expand Down
5 changes: 5 additions & 0 deletions cpp/tests/bitset_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,17 @@
#include <bitset>
#include <cstdint>
#include <random>
#include <type_traits>
#include <vector>

#include "monoprop/Bitset.h"

using monoprop::Bitset;

// data() is one deducing-this member, so const-ness of the pointer is deduced rather than declared.
static_assert(std::is_same_v<decltype(std::declval<Bitset<70> &>().data()), uint64_t *>);
static_assert(std::is_same_v<decltype(std::declval<const Bitset<70> &>().data()), const uint64_t *>);

namespace {

template <size_t N>
Expand Down
5 changes: 5 additions & 0 deletions cpp/tests/mp_graph_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <boost/test/unit_test.hpp>

#include <stdexcept>
#include <type_traits>
#include <vector>

#include "GraphBuildHarness.h"
Expand All @@ -28,6 +29,10 @@ using test_utils::core_with_gate;
using test_utils::graph_with_gates;
using test_utils::layer_with_gate;

// get_layer() is one deducing-this member, so const-ness of the reference is deduced rather than declared.
static_assert(std::is_same_v<decltype(std::declval<MPGraph &>().get_layer(0)), Layer &>);
static_assert(std::is_same_v<decltype(std::declval<const MPGraph &>().get_layer(0)), const Layer &>);

BOOST_AUTO_TEST_CASE(mp_graph_slice_graph_heisenberg_prefix_no_contract) {
auto graph = graph_with_gates(/*schrodinger=*/false, 5); // layers_ = [0,1,2,3,4]
auto sliced = graph.slice_graph(3, /*contract=*/false);
Expand Down
13 changes: 13 additions & 0 deletions cpp/tests/simulator_copy_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ static_assert(std::is_copy_constructible_v<MonomialPropagator<8>>, "simulator mu
static_assert(std::is_move_constructible_v<MonomialPropagator<8>>, "simulator must stay movable");
static_assert(!std::is_copy_assignable_v<MonomialPropagator<8>>, "copy assignment stays deleted");

// mp_op()/indexing() are single deducing-this members. indexing() reaches its result through a
// unique_ptr, whose operator* hands back a mutable referent regardless of the owner's const-ness, so
// without the forward_like a const propagator would silently expose a writable index.
// `detail` is qualified: the two using-directives above make an unqualified one ambiguous.
static_assert(
std::is_same_v<decltype(std::declval<MonomialPropagator<8> &>().mp_op()), monoprop::detail::MPOperator<8> &>);
static_assert(std::is_same_v<decltype(std::declval<const MonomialPropagator<8> &>().mp_op()),
const monoprop::detail::MPOperator<8> &>);
static_assert(
std::is_same_v<decltype(std::declval<MonomialPropagator<8> &>().indexing()), monoprop::detail::OperatorIndex<8> &>);
static_assert(std::is_same_v<decltype(std::declval<const MonomialPropagator<8> &>().indexing()),
const monoprop::detail::OperatorIndex<8> &>);

BOOST_FIXTURE_TEST_CASE(copy_constructed_simulator_matches_energy, ExampleDataFix) {
SimulatorConfig cfg{.comm = MPI_COMM_SELF};
auto sim = build_simulator<n_modes>(data, cfg);
Expand Down
Loading