spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
558 stars 164 forks source link

Documentation issues Error Initialization and Reset anomaly #7234

Closed stscijgbot-jp closed 1 year ago

stscijgbot-jp commented 1 year ago

Issue JP-2595 was created on JIRA by Rosa Diaz:

I found some issues with the documentation in read-the-docs and JDox

Read-the-docs

The Reset anomaly correction step is missing in the table at https://jwst-pipeline.readthedocs.io/en/stable/jwst/pipeline/calwebb_detector1.html#calwebb-detector1

the DQ-initializatio step does not mention that is there where the error is initialized. I believe it was mentioned before but now I cannot find it. This pseudo step is mentioned in the JDox, so there should be a mention of it in read-the-docs (more on JDox changes below)

JDox:

"Error initialization" section. It should be indicated in here that there is no "Error Initialization" step per-se, but rather initialized in the DQ-init step. Then the rest of the current paragraph should follow.

 

 

stscijgbot-jp commented 1 year ago

Comment by Alicia Canipe on JIRA:

Tagging Anton Koekemoer as well for the JDox update needed

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

The reset step was added to the Detector1 pipeline docs in #6785 via JP-2539.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Regarding error initialization, the only thing related to this that was ever done in the dq_init step was to simply populate the incoming empty ERR array with zeros, so that it could be added to by subsequent steps. The step itself doesn't even do that anymore, because it's taken care of automatically when any uncal file is loaded into a RampModel datamodel (the datamodels infrastructure automatically inflates and initializes the data arrays for any missing/empty extensions).

The real, first setting of any variance/error values doesn't take place until the ramp_fitting step. Coming out of that step, you will have fully-populated variance arrays due to read noise and Poisson noise, and the ERR array that's computed from the combination of those two variances.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

There is an entire top-level section of RTD devoted to explaining how and where the variance and error arrays are updated throughout the stages of pipeline processing: https://jwst-pipeline.readthedocs.io/en/latest/jwst/error_propagation/main.html  I will review these to make sure they are up to date with the current steps and pipelines.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

I've made an update in #6994 to add the resample step to the Error Propagation docs, because it uses the VAR_RNOISE array when applying IVM weighting. I've also verified that all of the other info in that section of RTD is accurate and up to date.

So at this point I think the only remaining work is on the JDox side. At least one update that's needed is to remove the "Error Initialization" step box from the flow-chart of Detector1 (Stage 1) pipeline steps. It's not a specific, stand alone step - the errors are computed in multiple steps.

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Should I reassign this back to Anton Koekemoer or Alicia Canipe or Karl Gordon to update the JDox side of things?

stscijgbot-jp commented 1 year ago

Comment by Rosa Diaz on JIRA:

Is this something that applies now or should be in Build 9.0? If it is for this build I will make the update (it will be faster), but if it is for Build 9.0, then assign it to Alicia and she will re-assign to the right person.

stscijgbot-jp commented 1 year ago

Comment by Alicia Canipe on JIRA:

Reassigned to Anton for the Error Initialization update in JDox https://jwst-docs.stsci.edu/jwst-science-calibration-pipeline-overview/stages-of-jwst-data-processing/calwebb_detector1#calwebb_detector1-Errorinitialization 

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Rosa Diaz these updates apply to the current build in Ops, as well as several builds before that. The error propagation hasn't really changed much in quite a while, other than the update to resample for it to use the VAR_RNOISE array (and that was implemented 1 or 2 builds ago).

stscijgbot-jp commented 1 year ago

Comment by Rosa Diaz on JIRA:

I am replacing the diagram now, but what about the description below for "Error initialization". Should it rather say "Error Propagation."?

Anton Koekemoer any recommendation about it.

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Thanks! The list of descriptions generally match the names of the steps, so in that sense the title of the step description would remain as "Error Initialization" (since that's what this step in the pipeline does, ie it initializes the errors).

However the actual text in that description then seems to talk more about propagation, and not really how they are initialized. The comment from Howard earlier in this ticket is actually pretty helpful in that context, in terms of describing how the errors are initialized, so maybe this could be included in the description for that "Error Initialization" step (or at least used as a starting point):

Regarding error initialization, the only thing related to this that was ever done in the dq_init step was to simply populate the incoming empty ERR array with zeros, so that it could be added to by subsequent steps. The step itself doesn't even do that anymore, because it's taken care of automatically when any uncal file is loaded into a RampModel datamodel (the datamodels infrastructure automatically inflates and initializes the data arrays for any missing/empty extensions).

The real, first setting of any variance/error values doesn't take place until the ramp_fitting step. Coming out of that step, you will have fully-populated variance arrays due to read noise and Poisson noise, and the ERR array that's computed from the combination of those two variances.

Would you like me to have a go at including that in the text? (if so just paste here please the actual link I'd need to click to make sure I edit the right version in jdox)

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Also, a question now (perhaps for Howard Bushouse?) – when I look at the readthedocs page which normally lists all the individual steps in the menu on the left hand side in alphabetical order ([https://jwst-pipeline.readthedocs.io/en/latest/jwst/pipeline/main.html)] I see the other steps in the figure all listed there (eg Group Scale Correction, Data Quality Initialization, etc), but 'Error Initialization' is not among that list (it's also not in the "package index" page https://jwst-pipeline.readthedocs.io/en/latest/jwst/package_index.html]) - is there something that needs to be set in readthedocs to include 'Error Initialization' there to help make it a bit easier to find?

stscijgbot-jp commented 1 year ago

Comment by Howard Bushouse on JIRA:

Anton Koekemoer that's because there is no "error initialization" step anywhere in the pipeline. Each of the other calibration steps either creates the variances/errors or updates them as necessary. As mentioned above, the first step in the pipeline to do this (currently) is the ramp_fit step in calwebb_detector1. Up to that point all of the variance and error arrays in any products are full of zeros. That's why I indicated that the box in the graphic in JDox that implies the existence of "Error Initialization" should be removed. I believe it's a holdover from years ago when the original pipeline design envisioned that we would have some kind of step like that, but the baseline design then ended up without it and having ramp_fit do the first population of any errors. Karl Gordon might remember more of the history that got us to this point.

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

ok, thanks Howard Bushouse , that clarifies it a lot more, thanks very much!

In that case I'm fine with implementing your recommendation of removing 'Error Initialization' as a heading here, and instead rather moving the text about error arrays to the section that discusses the 'Slope Fitting' step, as you suggest (and/or the 'Data Quality Initialization' step, since both of these steps are involved with the error array values)

stscijgbot-jp commented 1 year ago

Comment by Anton Koekemoer on JIRA:

Also, another question for Howard Bushouse, in terms of which pixels are eventually set to 0 in the ERR array for certain DQ flags -- could you point us perhaps please to a page (if it exists) that shows the flow of pixels flagged at different steps in the pipeline, in terms of how the user would know which DQ flags lead to a pixel being set to 0 in the ERR array?

That would be helpful in informing users about what types of pixels are currently excluded or included in the final stage3 products.

stscijgbot-jp commented 1 year ago

Comment by Karl Gordon on JIRA:

I agree with removing the error initialization step in the docs. Can provide an updated flow diagram.

Note that the treatment of uncertainties in the pipeline will be discussed in a section currently called "Meta-steps" in the pipeline algorithm paper that is being drafted. Just to confirm that this "step" really is different than others.

stscijgbot-jp commented 1 year ago

Comment by Rosa Diaz on JIRA:

I already re-did the diagram removing this step. The only thing that was remaining was the text part

nden commented 1 year ago

JP ticket was resolved