Average equivalent q-vectors in explicit structure factor

[mlund]

Summary

• StructureFactorPBC and StructureFactorIPBC were adding each direction's intensity individually to SamplingPolicy, bypassing averaging of equivalent lattice vectors (e.g. [1,0,0], [0,1,0], [0,0,1]) within each configuration
• Adds averageByMagnitude() free function to group and average intensities by |q| magnitude before accumulating across configurations, matching the approach used in pripps (average_duplicates in Rust)
• Removes #pragma omp critical from the parallel hot loop by writing to pre-allocated vector indices

Test plan

• All existing scatter unit tests pass (StructureFactorPBC ×3 variants, StructureFactorIPBC, FormFactorAtomicConstant)
• Verify with a production simulation that S(q) output is unchanged

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the explicit structure factor sampling to average intensities for equivalent q-vectors (same |q|) within each configuration before accumulating into the sampler, and it removes an OpenMP critical section by writing per-iteration results into a preallocated array.

Changes:

• Added averageByMagnitude() helper to group (q_norm, intensity) pairs by rounded |q| and compute a per-bin mean.
• Refactored StructureFactorPBC::sample() and StructureFactorIPBC::sample() to compute intensities in parallel into a pre-sized vector, then average-by-|q| and call addSampling().
• Removed #pragma omp critical from the parallel hot loop (map updates now happen after the parallel region).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

p_max is an int coming from config (see analysis.cpp), and here it is cast to size_t to compute n. If p_max is negative, this will underflow to a huge size_t and attempt an enormous allocation even though the for (int p=1; p<=p_max; ++p) loop would do zero iterations. Consider validating q_multiplier/p_max > 0 in the constructor (throw) or early-return in sample() before computing n/allocating.

Suggested change

	{
	{
	if (p_max <= 0) {
	return;
	}

[Copilot]

Same issue as in StructureFactorPBC::sample(): p_max is an int and may be negative from configuration, but it is cast to size_t to compute n and size q_intensity. A negative p_max will underflow and can trigger an enormous allocation. Add validation (e.g., reject p_max <= 0) before casting/allocating.

[Copilot]

averageByMagnitude() duplicates the binning/rounding logic and hard-codes a default precision (10000) that is meant to match SamplingPolicy::precision. This creates a maintainability risk: if a different TSamplingPolicy is supplied (or SamplingPolicy's binning changes), duplicates may be grouped differently than addSampling() bins them, producing inconsistent results. Consider factoring the rounding/binning into a shared helper/constant, or plumb the precision/binner from the sampling policy into averageByMagnitude() so the grouping and accumulation always use the same binning rule.

[Copilot]

Accumulator::count is an int, but it is counting elements from a std::vector and later cast to T. Using std::size_t (or at least an unsigned integral type) would better match container sizes and avoid potential overflow if pairs ever grows large.