STIPS 2.3.1 bug fixes with the documentation updates - #204
Conversation
| ins.detector = self.detector.replace('WFI', 'SCA') | ||
| ins.detector = self.detector.replace('SCA', 'WFI') |
There was a problem hiding this comment.
This should be left as it was – before v2.1.0, STPSF used SCANN to refer to detectors instead of WFINN. The ins object here comes from STPSF. STIPS supports STPSF v2.0.0, so when that version is installed, we have to revert to calling detectors with "SCA" as this line did before this PR.
There was a problem hiding this comment.
The real bug happens with ins.psf_grid(). STPSF appends its internal detector name to the files generated with that function, so v2.0.0 will append "SCANN.fits". STIPS' AstroImage.make_epsf_array() looks for file names ending in "WFINN.fits". When it doesn't find them, it regenerates the PSF grid, even when psf_cache_enable is on. That causes the extra computation time experienced in #201.
I've added a commit to make the file name check in AstroImage.make_epsf_array() more backward compatible as well since we can't change the behavior of ins.psf_grid().
| ot['x'] = Column(data=xfs+self.out_origin, unit='pixel') | ||
| ot['y'] = Column(data=yfs+self.out_origin, unit='pixel') | ||
| ot['x_epsf'] = Column(data=xfs+self.out_origin, unit='pixel') | ||
| ot['y_epsf'] = Column(data=yfs+self.out_origin, unit='pixel') | ||
| ot['x_input'] = Column(data=self.out_origin, unit='pixel') | ||
| ot['y_input'] = Column(data=self.out_origin, unit='pixel') |
There was a problem hiding this comment.
One of these should remain as 'x' and 'y' instead of being moved to a different name in order to maintain backward compatibility in the headers. I will change 'x_epsf' and 'y_epsf' back and also edit the updated text in bugs.rst.
24fad5f to
88688a8
Compare
|
We would still like this release to start dynamically updating |
| self.photfnu = self.PHOTFNU[self.filter] | ||
| self.photplam = self.PHOTPLAM[self.filter] | ||
| self.custom_background *= self.convertToCounts('p') |
There was a problem hiding this comment.
This function is triggered when Instrument.reset() attempts to set self.background = self.pixel_background. (Users would encounter it when ObservationModule.nextObservation() callsInstrument.reset().)
In Instrument.reset(), self.photfnu and self.photplam are defined just after self.background, but this highlighted snippet needs them to exist beforehand because Instrument.convertToCounts() references them.
I edited Instrument.reset() to define self.photfnu and self.photplam before self.background, removing the need to set them again here in Instrument.pixel_background_unit().
There was a problem hiding this comment.
I also changed the log message to show the converted background value in counts/s.
There was a problem hiding this comment.
If quantum efficiency is always assumed to be 1 across the package, I do still wonder whether Instrument.convertToCounts() should have separate paths for photons and electrons. If not, should we make 'p' and 'e' equivalent in that function (with both being read as 'p'?) and note the change both in the docstring and in docs/using_stips/catalogue_formats.html (especially in the Mixed catalogs section)?
There was a problem hiding this comment.
I agree with you that we should make 'p' and 'e' equivalent in that function and note that STIPS assumes QE=1. But all together, I think we are also assuming counts=p=e as well with gain = 1 because in the validation code, we don't do the gain correction to compare the STIPS output to Pandeia (e- and e-/s).
There was a problem hiding this comment.
I've changed Instrument.convertToCounts() to treat 'e' like 'p' and edited the docs as well.
| conversion assumes a quantum yield of 1, where photons are equivalent to | ||
| electrons. | ||
|
|
||
| * the string value 'jbt', which will use the JBT background tool to |
There was a problem hiding this comment.
I'm not sure if the current STIPS is configured to call JBT. If yes, we want to change that to RBT but if not, I think we need to remove any mention of JBT.
There was a problem hiding this comment.
OK, I see the v2.0 changelog includes the removal of JBT. It's still mentioned in stips/data/stips_config.yaml, so I'll remove that as well as the mention in the documentation. observation_jbt_location should also be removed from this docs page as well.
There was a problem hiding this comment.
For Pandeia version later than 2025.9, we need this extra environment for Pandeia:
export PSF_DIR="<absolute_path_to_this_folder>/ref_data/pandeia_psfs"
|
Merging after successful test run and reaching consensus with @eunkyuh. |
Several bug fixes include:
Documentation updates include
Installation yml updates include pandeia.engine requirements to be 2025.9 or higher and allowing newer versions of poppy than 1.0.3. The utilities infrastructure has also been updated so it can work with newer versions of Pandeia.
Implemented dynamic calculation of zeropoints based on WFI throughput files from the user's local Pandeia reference data. This should help mitigate output flux mismatches between STIPS and Pandeia if a user has installed an unexpected version of the latter. The calculated values from the current Pandeia reference data match those found in the Roman Technical Repo.