convergence test: document the weight-order trap (documentation only; no functional change)

[oshaughnessy-junior]

Documentation only. No functional change ships here: the two samplers differ from rift_O4d by a corrected comment, and one deprecation note in helper_LDG_Events.py. An earlier revision of this branch carried a draw-order reorder in convergence_test_NormalSubIntegrals; it was withdrawn after review, for the reasons below.

The trap

convergence_test_NormalSubIntegrals splits _rvs into ncopies CONTIGUOUS segments and assumes they are independent. They are not: the save-P thresholding block (mcsampler.py:739-752) reindexes every _rvs column by cumulative-weight order, so surviving rows are sorted by weight ascending.

segments	max/min segment mass
weight-ordered (what the test sees)	1.15e3
draw-ordered	1.83

Two candidate repairs, both of which fail

1. The in-tree workaround does nothing. mcsampler.py:1138 / mcsamplerGPU.py:1559 said "rvs['weights'] is sorted ... Recalculated weights are not. Use explicitly calculated weights until sorting effect identified." Measured, cached and recomputed weights are both 100% ascending -- the components were permuted by the same reindexing. That claim was false and is removed.

2. Reordering by sample_n also fails -- this is what the review caught, plus a second mode found while checking it:

after appending, before the threshold block:  len(integrand)=3000  len(sample_n)=1500   MISMATCH
after the block re-runs:                      len(integrand)=2999  len(sample_n)=2999   ok

In that window, numpy fancy-indexing silently returns the shorter array and drops every new sample. And even once the lengths agree, sample_n is arange(len(...)) assigned to an already-permuted array, so it encodes the order at the start of that call:

after two integrate() calls	ascending	segment max/min
weight-ordered	1.000	1.12e3
reordered by sample_n	0.752	373
single call, for reference	0.491	1.83

So the reorder was partly ineffective even with a length guard.

Why nothing is fixed here

A real repair needs stable ids assigned where samples are appended, in every sampler. This test has no live callers -- every production call site is commented out, only test/test_like_and_samp.py uses it -- so that plumbing is not justified. Since sorting here also drives output downselection, the conservative option was preferred.

What ships

• Corrected comments in both samplers: the false claim removed, both failure modes and the required plumbing stated.
• test_convergence_sample_order.py, 5 checks, holding the measurements. test_sample_n_does_not_encode_draw_order_after_reuse is written to FAIL if the samplers ever grow stable append-time ids -- the signal to revisit the note rather than let it rot.
• Deprecation note on the bayestar-skymap branch of helper_LDG_Events.py: unused in production ~a decade, and the only place --n-chunk drops to 500, which makes it the least-tested regime for anything chunk-scaled (--portfolio-weight-clip uses tau = C*sqrt(n_chunk)*mean(w), ~4.5x more aggressive there than at the n_chunk=1e4 all clip testing used).

CI: the two failing container canaries are unrelated upstream pygsl-lite/setuptools failures.

[oshaughnessy-junior]

You were right, and checking it turned up a second problem in the same place -- so I have withdrawn the source hunks. The diff against rift_O4d in mcsampler.py / mcsamplerGPU.py is now comment-only.

[P2] confirmed

Measured the two-integrate() scenario directly:

after appending, before the threshold block:  len(integrand)=3000  len(sample_n)=1500   MISMATCH
after the block re-runs:                      len(integrand)=2999  len(sample_n)=2999   ok

So in the window you identified, numpy fancy-indexing silently returns the shorter array and every new sample is dropped. Exactly as described.

A second problem, which is what settled it

Even once the block re-runs and the lengths agree, reordering by sample_n does not restore draw order. It is (re)created as arange(len(...)) against an already-permuted array, so it encodes the order at the start of that call:

after two integrate() calls	ascending fraction	segment max/min mass
weight-ordered	1.000	1.12e3
reordered by sample_n	0.752	373
(single call, for reference)	0.491	1.83

So the recommended fix would have been partly ineffective even with a length guard. Getting it right needs stable ids assigned where samples are APPENDED, in every sampler -- and this test has no live callers (all production call sites commented out; only test/test_like_and_samp.py), so that plumbing is not justified.

What ships instead

• The false claim is removed. The old note said recomputing the weights from the components avoids the sorting; measured, cached and recomputed weights are both 100% ascending, because the components were permuted by the same reindexing.
• The caveat now states what is actually true, including both reuse failure modes and what anyone re-enabling the test must do first.
• Tests keep the evidence: 5 checks, including both reuse modes. test_sample_n_does_not_encode_draw_order_after_reuse is written to fail if the samplers ever grow stable append-time ids -- that is the signal to revisit the caveat rather than leave it stale.

On "silly to keep a dead array": agreed, and that is roughly why this ended where it did. sample_n is not currently a usable draw-order record, and making it one is real work whose only beneficiary is a test nobody runs. Documenting the trap costs nothing and does not touch a shipped code path.

Also in this branch: the skymap branch flagged deprecated

Unrelated to the ordering issue, surfaced while auditing chunk-size-dependent behaviour. The bayestar-skymap path is unused in production for ~a decade and is the only place --n-chunk drops to 500 -- which makes it the least-tested regime for anything scaling with the chunk size. In particular --portfolio-weight-clip uses tau = C*sqrt(n_chunk)*mean(w), so it is ~4.5x more aggressive there than at the n_chunk=1e4 used for all clip testing. Flagged in place, with a note not to treat clip validation as covering it.

CI: agreed the two failing container canaries are unrelated upstream pygsl-lite/setuptools failures.