Assorted small cleanups: Circuit mapping recomputation, scatter_state_rows_from_, VecD(0)

[robertodr]

🤖 AI text below 🤖

A small basket of readability cleanups found during a review pass. None change behaviour; each is
independent, so they can be split off if any is contentious.

1. Circuit re-derives and re-validates its parameter mapping on every read

src/monoprop/circuit.py:243

self.resolved_mapping  # validates the per-gate index scheme

A bare expression statement used for its side effect. It reads as dead code — this is exactly what
ruff's B018 (useless-expression) catches, and B (flake8-bugbear) is not in the enabled rule set,
which is why it slips through. Make the validation an explicit call.

Related, in the same class: resolved_mapping is a property that rebuilds the tuple and re-runs
validate_parameter_mapping on every access, and it is read from n_parameters (itself a
property), from __add__ twice, and from MonomialPropagator.build_graph, which reads both
resolved_mapping and n_parameters. Circuit is validated at construction and gates is a tuple,
so computing the mapping once in __init__ and having the property return the stored value is
consistent with how the class already treats itself as frozen. (Note this does make the "frozen after
construction" assumption explicit rather than incidental — worth a line in the docstring.)

Also in __init__: _is_identity_gate is evaluated up to three times per gate — once inside
any(...), once in the kept comprehension, once in the gates comprehension. Compute the predicate
once.

2. scatter_state_rows_from_ recomputes a distance inside its loop

cpp/monoprop/detail/operator/MPOperator.h:258-264

const auto first = std::ranges::lower_bound(state_rows_, static_cast<TermIndex>(first_row));
for (auto it = first; it != state_rows_.end(); ++it) {
    out[*it] = state_vals_[static_cast<size_t>(std::distance(state_rows_.begin(), it))];
}

std::distance is O(1) on a random-access iterator, so this is not a performance problem — but a
running index says what is meant more directly:

for (size_t k = static_cast<size_t>(first - state_rows_.begin()); k < state_rows_.size(); ++k) {
    out[state_rows_[k]] = state_vals_[k];
}

3. VecD(0) where an empty vector is meant

cpp/monoprop/MPFunctions.cpp, in ev_and_grad_impl:

return {e_core + mpi::allreduce_sum(state.dot(op), comm), VecD(0)};

VecD(0) is a zero-length vector, but it reads as "a vector containing 0". VecD{} is unambiguous.

4. Optional, lower priority: all_pairwise_commute() is O(terms²)

src/monoprop/majorana.py and src/monoprop/pauli.py both build a per-term set/dict and then walk
itertools.combinations(..., 2). It runs inside every ExpGate.__init__
(src/monoprop/circuit.py:111), so it is on the authoring path for every gate.

This is fine for the small commuting generators that are typical, and the semantics require an
all-pairs check, so this is a "note it" rather than a "fix it now" — but it is quadratic for a large
commuting group, and a bitmask formulation would make it cheap if that ever shows up in a profile.

Verification

just test and ruff check / ruff format --check; no behavioural change is intended by any of
these, so a green suite is the whole bar. Item 1's caching change is the only one with any semantic
content (it fixes the mapping at construction time), so it wants a test asserting that a Circuit's
resolved_mapping and n_parameters still agree after __add__.

---

Found by a code-reading review of the repository at 29a8050.