RomanNoiseBuilder broke CI

[arunkannawadi]

Merging #66 and the corresponding changes in roman_imsim_testdata left the end-to-end CI test broken. This needs to be fixed before other features can be merged confidently.

[arunkannawadi]

Logs for reference: https://github.com/DukeCosmology/roman_imsim/actions/runs/22427309461

[aguinot]

Ok so I think it is just a miss match in the configuration.
I see that the reference image contains noise. Do we want to drasw the image with the noise? Right now the test is configured with the NoNoise builder see here.
I would also highly recomend to store the full config file of the reference image. That would help a lot to debug this kind of issue. And maybe a commit number or some information about the state of the repo that was used to create the reference image.

[sidneymau]

The config file is the hack.yaml file, so I'm a little confused. I guess someone updated one without the other?

[sidneymau]

And maybe a commit number or some information about the state of the repo that was used to create the reference image.

We do have this too -- the images are subject to the same version control as everything else. Looking at the [version history], it looks like the reference images weren't rerun with #66.

edit: oh, I guess you mean of roman_imsim?

[sidneymau]

It's not guaranteed that whomever is updating the reference image will have installed roman_imsim from source locally vs. from pip. The version numbering is pretty coarse right now --

>>> importlib.metadata.version("roman_imsim")
'2.0'

I'll make a PR to update the versioning so we can get something a bit more informative for this

[aguinot]

I think I traced back the potential origin of the confusion.
There has been some modifications to how the noise was handled in roman_imsim before my PR on the "revision" of noise builder was merged but after I opened it.
The reference image was last changed in PR #5 but by looking at the history of the hack.yaml it hasn't been updated at the same time. I tried to propagate the modification discussed in issue #68 but I don't know exactly how it was done.
I re-run on my side with the noise parameters as follow:

    noise:
        # These are all by default turned on, but you can turn any of them off if desired:
        type: RomanNoise
        mjd: { type: ObSeqData, field: mjd }
        stray_light: True
        thermal_background: True
        reciprocity_failure: True
        dark_current: True
        nonlinearity: True
        ipc: True
        read_noise: True
        sky_subtract: False

But it still does not match 😕
It is not because of sky_subtract: False. I am pretty sure that the current reference image has the sky background not subtracted.

[sidneymau]

See #94 for dynamic versioning. This should give us something that we can at least reference when figuring out which version of roman_imsim was used in future runs

[aguinot]

See #94 for dynamic versioning. This should give us something that we can at least reference when figuring out which version of roman_imsim was used in future runs

Your PR gave me an idea (maybe it was yours too). We could add the version of roman_imsim in the header of the simulated image.

[sidneymau]

But it still does not match 😕
It is not because of sky_subtract: False. I am pretty sure that the current reference image has the sky background not subtracted.

I think there's also a difference of int vs. float -- the current reference image are ints, whereas the outputs we are producing are floats, at least on my end 🤔

[aguinot]

You have float because you don't use the noise so the image is not quantized. From my investigation, I think it is when because we use photon shooting with chromatic objects. Photons get non-integer flux value and you end up with floats in the image. I guess it is to respect the requiered flux and SED normalization with photon that have are monochromatic (single wavelength). You can see the full discussion about this on Slack here.

[sidneymau]

I think my standing question is which is the correct state of the reference images and config right now? If the quantization is desired, then we don't have a config that produces that, from what I can see

[aguinot]

The quantization is added with the noise. The current config does not add noise. I re-run with my update of the noise builder and I don't get the same imge.

[aguinot]

Ok so if we merge PR #9 in roman_imsim_testdata, it should fix the CI tests.