Resolve !SIM.random.seed per apply_to call, not once

[astronomyk]

What

Two noise effects resolved !SIM.random.seed in a way that destroys the reference on first use:

• ShotNoise.apply_to wrote the resolved integer back into self.meta["random_seed"];
• PoorMansHxRGReadoutNoise.apply_to did the same, and additionally rebaked its whole self.meta through from_currsys.

After the first readout, the effect's meta holds a frozen integer instead of "!SIM.random.seed", so any later readout from a reused OpticalTrain re-seeds with the first exposure's seed, regardless of what !SIM.random.seed was updated to in cmds.

This PR resolves into locals instead — exactly the pattern BasicReadoutNoise and PixelResponseNonUniformity already use. First-call behaviour, resolution order and generated values are unchanged; only repeated apply_to calls now honour a per-readout seed.

Why it matters

readout() deliberately supports repeated calls (it deep-copies results so "subsequent readouts" don't clobber earlier ones — optical_train.py), and repeated readout from one observe() is the natural way to generate N darks or a detector-linearity DIT ramp: measured on METIS, ~1–2 s per additional frame instead of ~60 s for a full rebuild+observe.

Today that pattern is statistically correct but impossible to make reproducible: readouts 2..N carry the frame-1 readout-noise pattern rolled to random offsets (measured: σ agrees with independent simulation to 4×10⁻⁷, but ~92% of pixels differ and reruns don't reproduce). With this fix, one-observe/N-readouts at per-frame seeds is bit-for-bit identical to N independent simulations.

Validation (METIS_Simulations differential harness, per-pixel comparison)

Check	Result
Single-frame no-regression: darkLM.yaml ×3 frames, patched vs unpatched main	3/3 frames BITWISE IDENTICAL (all 4,194,304 px each)
One observe + 3 readouts at per-frame seeds vs 3 independently simulated frames (patched)	3/3 frames BITWISE IDENTICAL — repeated readout is now exactly reproducible

Notes:

• np.random.seed (process-global, legacy RNG) is untouched here — moving to per-effect Generators spawned from a SeedSequence would be the fuller modernisation, but is a behaviour-changing refactor best done separately. This PR is the minimal fix that makes repeated readout deterministic.
• Full measurement log: METIS_Simulations_Deux findings F-D18/F-D24/F-D34.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.88%. Comparing base (608adfa) to head (0b1b064).

Files with missing lines	Patch %	Lines
scopesim/effects/electronic/noise.py	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #974      +/-   ##
==========================================
- Coverage   75.89%   75.88%   -0.01%     
==========================================
  Files          70       70              
  Lines        9030     9029       -1     
==========================================
- Hits         6853     6852       -1     
  Misses       2177     2177              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[astronomyk]

Another one of these super-low hanging fruit that will help massively speed up METIS_Simulations. Given that BasicReadoutNoise and PixelResponseNonUniformity also use this pattern, it should be fine to switch to this, imo. I'd be interested to hear your opinions. Thanks!

[oczoske]

That Claude sure sounds like a smug, pompous arsehole.

I'm not as clever and don't use Scopesim with a seed. Just looking at the code I'm wondering whether this really changes anything - why would !SIM.random_seed change between readouts?

[hugobuddel]

Can you please explain as a human why this change is necessary @astronomy,k ?

Did you read yourself what your LLM wrote and did you agree with it?

Does this do what we actually want it to do?

What is it that you want to happen?

The title seems to indicate that this resolves an open ticket, does it? If so, which one?

I'd say we like to have determinism, but not have every readout to be identical, that is, calling readout() multiple times, should give different results. Does this change achieve that?

The LLM says

moving to per-effect Generators spawned from a SeedSequence would be the fuller modernisation

this is also what we agreed upon, so why not just do that

[hugobuddel]

In general, it would be much preferable if you talk to us, and not let your bot talk to us.

[teutoburg]

The LLM says

moving to per-effect Generators spawned from a SeedSequence would be the fuller modernisation

this is also what we agreed upon, so why not just do that

see also #298

[astronomyk]

I see I'm hitting some nerves here - my apologies. To be fair, it shouldn't be me who is looking into ways to simplify and speed up our systems, but here we are. It's impossible for anyone to run METIS_Simulation locally and generate the necessary data for testing recipes and not waste a day waiting for 95% of the computational effort to be thrown away because it wasn't needed in the first place. I don't have that kind of time to waste, so you'll have to excuse me using the toaster to save time. If I'm doing this as a last resort. I keep trying to give my job to someone else so I can actually get back to coding like a you guys, but no-one wants it ...

</rant>

Here's the background - https://github.com/AstarVienna/METIS_Simulations/blob/main/YAML/ESO/detlinIFU.yaml

To generate realistic noise that changes over multiple frames for the Dark and Linearity recipes of the METIS pipeline, the driving yaml file currently runs 32 full simulations from scratch. Incredibly wasteful when we have the .readout method on the Detector object to save exactly this from happening. However on instantiation, the noise Effects overwrite their "!SIM.random.seed" reference string in the meta dict, and so you can't just update the bang-string between each Detector.readout call. METIS_Simulations would need a hack to update metis[<effect_name>].meta["random_seed"] which is even more undesireable than this update.

This minor update fixes this problem so that METIS_Simulations can make use of the Detector.readout funcitonality properly.

[astronomyk]

And on your comment here -

The LLM says

moving to per-effect Generators spawned from a SeedSequence would be the fuller modernisation

this is also what we agreed upon, so why not just do that

It also says:

but is a behaviour-changing refactor best done separately. This PR is the minimal fix that makes repeated readout deterministic.

So re SeesSequencers, yes, please do! I don't care who does it, it is a very sound solution.
But is this going to happen today? I doubt it. That issue has been open for 3 years. I've spent this week together with Janus getting the pipeline into a state to deliver to ESO on Monday. The simulations are the biggest time sink in getting things working.

This tiny fix hurts no-one and allows me to actually do what I need to get done

[teutoburg]

What's wrong with just setting the seed to None? That should re-randomise every time right?

[hugobuddel]

Thank you for the context @astronomyk, that is exactly what was missing and that would be impossible to derive from the LLM description, while I would consider it the most important information. Sorry for being rude.

I wasn't aware about the delivery this Monday, sorry, that was my fault and I dropped the ball again (w.r.t. the archive).

Back to this P.R. I think saving the seed is already a hack to ensure the randomness is 'correct'. I guess to ensure that each detector (for e.g. MICADO or METIS IFU) has a different seed, so they don't look all the same. Is that behavior kept? Edit: but it could be for some other reason, not sure, but it is probably deliberate.

[hugobuddel]

What's wrong with just setting the seed to None? That should re-randomise every time right?

Then the simulations are not deterministic anymore, which is something we want for the delivery.

[oczoske]

I've just checked with !SIM.random.seed set to a number (None is default), and indeed I get identical results when I readout twice from the same observe(). That would be a very serious issue, which is not solved by this change (which in itself seems okay to minimise interference between multiple instances of the effect - parallelisation comes to mind). The problem is that the rngs are initialised (to the same seed) every time that apply_to is called. The solution might be to move that to __init__.

See also #97

[teutoburg]

What's wrong with just setting the seed to None? That should re-randomise every time right?

Then the simulations are not deterministic anymore, which is something we want for the delivery.

Ah, sure, that makes sense 👍

[teutoburg]

I've just checked with !SIM.random.seed set to a number (None is default), and indeed I get identical results when I readout twice from the same observe(). That would be a very serious issue, which is not solved by this change (which in itself seems okay to minimise interference between multiple instances of the effect - parallelisation comes to mind). The problem is that the rngs are initialised (to the same seed) every time that apply_to is called. The solution might be to move that to __init__.

See also #97

I think this would all improve by using to the Generator feature mentioned in #298. I now regret not doing that earlier, it's been in the back of my head for years...

[hugobuddel]

I've just checked with !SIM.random.seed set to a number (None is default), and indeed I get identical results when I readout twice from the same observe()

My suggestion has always been to use the readout time as a random seed (or include that in the seed), and conceptually, calling readout() again would require a new readout time.

But it is difficult to get all considerations into account; and I fear this P.R. will break something else.

[astronomyk]

I'm happy to ask the Toaster for a suggestion for implementing the SeedSequencer, if we as a collective, are open to reading it. It's cheap to do so, so it may be worth doing simply to get inspiration....