Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add suffix to Step.spec #1347

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ general
- Refactor general step input handling to avoid early closing of
input files to allow using more lazy loading [#1342]

- Provide suffix in Step.spec for steps to allow default suffix use
within and outside Pipeline runs [#1347]

Comment on lines +54 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Provide suffix in Step.spec for steps to allow default suffix use
within and outside Pipeline runs [#1347]

now that #1375 is merged (switching change log handling to towncrier) this change log entry should be a file in changes/ instead:

echo "Provide suffix in Step.spec for steps to allow default suffix use within and outside Pipeline runs" > changes/1347.general.rst

(this is just because #1375 was merged while active PRs existed; new PRs will have these instructions in their checklist)



source_catalog
Expand Down
6 changes: 3 additions & 3 deletions docs/roman/data_products/product_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ the user is running the pipeline. The input for each optional step is the output
+------------------------------------------------+-----------------+--------------------------+------------------+---------------------+---------------------------------------+
| :ref:`source_detection <source_detection_step>`| | sourcedetectionstep (opt)| ImageModel | DN/s | Sources identified in the data |
+------------------------------------------------+-----------------+--------------------------+------------------+---------------------+---------------------------------------+
| :ref:`tweakreg <tweakreg_step>` | | tweakregstep (opt) | ImageModel | DN/s | WCS aligned with GAIA |
| :ref:`tweakreg <tweakreg_step>` | | tweakreg (opt) | ImageModel | DN/s | WCS aligned with GAIA |
+------------------------------------------------+-----------------+--------------------------+------------------+---------------------+---------------------------------------+
| | | cal | ImageModel | DN/s | 2-D calibrated exposure data |
+------------------------------------------------+-----------------+--------------------------+------------------+---------------------+---------------------------------------+
Expand All @@ -73,9 +73,9 @@ the user is running the pipeline. The input for each optional step is the output
+---------------------------------------------------+-----------------+------------------------------+------------------+---------------------+---------------------------------------+
| :ref:`sky_match <skymatch_step>` | asn | skymatch (opt) | ModelLibrary | MJy/sr | A list of _cal files |
+---------------------------------------------------+-----------------+------------------------------+------------------+---------------------+---------------------------------------+
| :ref:`outlier_detection <outlier_detection_step>` | | outlier_detection_step (opt) | ModelLibrary | MJy/sr | A list of _cal files |
| :ref:`outlier_detection <outlier_detection_step>` | | outlier_detection (opt) | ModelLibrary | MJy/sr | A list of _cal files |
+---------------------------------------------------+-----------------+------------------------------+------------------+---------------------+---------------------------------------+
| :ref:`resample <resample_step>` | | resamplestep (opt) | ModelLibrary | MJy/sr | A list of _cal files |
| :ref:`resample <resample_step>` | | resample (opt) | ModelLibrary | MJy/sr | A list of _cal files |
+---------------------------------------------------+-----------------+------------------------------+------------------+---------------------+---------------------------------------+
| | | i2d | MosaicModel | MJy/sr | A 2D resampled image |
+---------------------------------------------------+-----------------+------------------------------+------------------+---------------------+---------------------------------------+
10 changes: 4 additions & 6 deletions romancal/assign_wcs/assign_wcs_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ class AssignWcsStep(RomanStep):

reference_file_types = ["distortion"]

spec = """
suffix = string(default="assignwcs")
"""

def process(self, input):
reference_file_names = {}
if isinstance(input, rdm.DataModel):
Expand All @@ -49,12 +53,6 @@ def process(self, input):
log.info("Using reference files: %s for assign_wcs", reference_file_names)
result = load_wcs(input_model, reference_file_names)

if self.save_results:
try:
self.suffix = "assignwcs"
except AttributeError:
self["suffix"] = "assignwcs"

return result


Expand Down
11 changes: 3 additions & 8 deletions romancal/dark_current/dark_current_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ class DarkCurrentStep(RomanStep):
dark current reference data from the input science data model.
"""

reference_file_types = ["dark"]

spec = """
suffix = string(default="darkcurrent")
dark_output = output_file(default = None) # Dark corrected model
"""

reference_file_types = ["dark"]

def process(self, input):
if isinstance(input, rdm.DataModel):
input_model = input
Expand Down Expand Up @@ -63,10 +64,4 @@ def process(self, input):
dark_model.save(self.dark_output)
# not clear to me that this makes any sense for Roman

if self.save_results:
try:
self.suffix = "darkcurrent"
except AttributeError:
self["suffix"] = "darkcurrent"

return out_model
10 changes: 4 additions & 6 deletions romancal/dq_init/dq_init_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ class DQInitStep(RomanStep):

reference_file_types = ["mask"]

spec = """
suffix = string(default="dqinit")
"""

def process(self, input):
"""Perform the dq_init calibration step

Expand Down Expand Up @@ -99,10 +103,4 @@ def process(self, input):
except AttributeError:
pass

if self.save_results:
try:
self.suffix = "dqinit"
except AttributeError:
self["suffix"] = "dqinit"

return output_model
10 changes: 4 additions & 6 deletions romancal/flatfield/flat_field_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class FlatFieldStep(RomanStep):

reference_file_types = ["flat"]

spec = """
suffix = string(default="flat")
"""

def process(self, input_model):
if not isinstance(input_model, rdm.DataModel):
input_model = rdm.open(input_model)
Expand Down Expand Up @@ -44,10 +48,4 @@ def process(self, input_model):
if reference_file_model is not None:
reference_file_model.close()

if self.save_results:
try:
self.suffix = "flat"
except AttributeError:
self["suffix"] = "flat"

return output_model
3 changes: 2 additions & 1 deletion romancal/flux/flux_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class FluxStep(RomanStep):
""" # noqa: E501

spec = """
""" # noqa: E501
suffix = string(default="flux")
"""

reference_file_types = []

Expand Down
7 changes: 1 addition & 6 deletions romancal/jump/jump_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class JumpStep(RomanStep):
sat_required_snowball = boolean(default=True) # Require the center of snowballs to be saturated
expand_large_events = boolean(default=False) # must be True to trigger snowball and shower flagging
use_ramp_jump_detection = boolean(default=True) # Use jump detection during ramp fitting
suffix = string(default="jump")
""" # noqa: E501

reference_file_types = ["gain", "readnoise"]
Expand Down Expand Up @@ -163,10 +164,4 @@ def process(self, input):

result.meta.cal_step.jump = "COMPLETE"

if self.save_results:
try:
self.suffix = "jump"
except AttributeError:
self["suffix"] = "jump"

return result
9 changes: 4 additions & 5 deletions romancal/linearity/linearity_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class LinearityStep(RomanStep):

reference_file_types = ["linearity"]

spec = """
suffix = string(default="linearity")
"""

def process(self, input):
# Open the input data model
if isinstance(input, rdd.DataModel):
Expand Down Expand Up @@ -67,9 +71,4 @@ def process(self, input):
# Update the step status
input_model.meta.cal_step["linearity"] = "COMPLETE"

if self.save_results:
try:
self.suffix = "linearity"
except AttributeError:
self["suffix"] = "linearity"
return input_model
1 change: 1 addition & 0 deletions romancal/outlier_detection/outlier_detection_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class OutlierDetectionStep(RomanStep):
good_bits = string(default="~DO_NOT_USE+NON_SCIENCE") # DQ bit value to be considered 'good'
allowed_memory = float(default=None) # Fraction of memory to use for the combined image
in_memory = boolean(default=False) # Specifies whether or not to keep all intermediate products and datamodels in memory
suffix = string(default="outlier_detection")
""" # noqa: E501

def process(self, input_models):
Expand Down
10 changes: 4 additions & 6 deletions romancal/photom/photom_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class PhotomStep(RomanStep):

reference_file_types = ["photom"]

spec = """
suffix = string(default="photom")
"""

def process(self, input):
"""Perform the photometric calibration step

Expand Down Expand Up @@ -68,10 +72,4 @@ def process(self, input):
input_model.meta.cal_step.photom = "SKIPPED"
output_model = input_model

if self.save_results:
try:
self.suffix = "photom"
except AttributeError:
self["suffix"] = "photom"

return output_model
2 changes: 0 additions & 2 deletions romancal/pipeline/exposure_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ def process(self, input):

log.info(f"Processing a WFI exposure {input_filename}")

self.dq_init.suffix = "dq_init"
result = self.dq_init(input)

if input_filename:
Expand Down Expand Up @@ -177,7 +176,6 @@ def process(self, input):
result.meta.cal_step.source_detection = "SKIPPED"
result.meta.cal_step.tweakreg = "SKIPPED"

self.output_use_model = True
results.append(result)

# Now that all the exposures are collated, run tweakreg
Expand Down
7 changes: 1 addition & 6 deletions romancal/pipeline/mosaic_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class MosaicPipeline(RomanPipeline):
spec = """
save_results = boolean(default=False)
on_disk = boolean(default=False)
suffix = string(default='i2d')
"""

# Define aliases to steps
Expand Down Expand Up @@ -71,11 +72,8 @@ def process(self, input):

if file_type == "asn":
input = ModelLibrary(input, on_disk=self.on_disk)
self.flux.suffix = "flux"
result = self.flux(input)
self.skymatch.suffix = "skymatch"
result = self.skymatch(result)
self.outlier_detection.suffix = "outlier_detection"
result = self.outlier_detection(result)
#
# check to see if the product name contains a skycell name & if true get the skycell record
Expand Down Expand Up @@ -126,7 +124,6 @@ def process(self, input):
self.resample.output_shape,
)
wcs_file = asdf.open(self.resample.output_wcs)
self.suffix = "i2d"
result = self.resample(result)
self.output_file = input.asn["products"][0]["name"]
# force the SourceCatalogStep to save the results
Expand All @@ -137,12 +134,10 @@ def process(self, input):
exit(0)

else:
self.resample.suffix = "i2d"
self.output_file = input.asn["products"][0]["name"]
result = self.resample(result)
self.sourcecatalog.save_results = True
result_catalog = self.sourcecatalog(result) # noqa: F841
self.suffix = "i2d"
if input_filename:
result.meta.filename = self.output_file

Expand Down
6 changes: 1 addition & 5 deletions romancal/refpix/refpix_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class RefPixStep(RomanStep):
# interpolation of the reference pixels
fft_interpolate = boolean(default=True) # Turn on or off the FFT interpolation
# of the reference pixel padded values.
suffix = string(default="refpix")
"""

reference_file_types = ["refpix"]
Expand Down Expand Up @@ -67,9 +68,4 @@ def process(self, input):
)
# Update the step status
output.meta.cal_step["refpix"] = "COMPLETE"
if self.save_results:
try:
self.suffix = "refpix"
except AttributeError:
self["suffix"] = "refpix"
return output
22 changes: 0 additions & 22 deletions romancal/regtest/test_dark_current.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,6 @@ def test_dark_current_outfile_step(rtdata, ignore_asdf_paths):
assert diff.identical, diff.report()


@pytest.mark.bigdata
def test_dark_current_outfile_suffix(rtdata, ignore_asdf_paths):
"""Function to run and compare Dark Current subtraction files. Here the
test is for renaming the output file."""
input_datafile = "r0000101001001001001_01101_0001_WFI01_linearity.asdf"
rtdata.get_data(f"WFI/image/{input_datafile}")
rtdata.input = input_datafile

args = [
"romancal.step.DarkCurrentStep",
rtdata.input,
"--output_file=Test_dark",
'--suffix="suffix_test"',
]
RomanStep.from_cmdline(args)
output = "Test_darkcurrent.asdf"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose to delete this test as it appears to check that calling the DarkCurrentStep and providing an expected output suffix of suffix_test produces a file with a suffix darkcurrent.

This isn't the expected behavior for stpipe (the output file should have the suffix provided on the commandline, suffix_test not darkcurrent).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddavis-stsci mentioned that this test was intended to test if a user provided suffix is used.

It could be made to pass by:

  • updating the expected output from Test_darkcurrent.asdf to Test_suffix_test.asdf (which includes the user-provided suffix)
  • update the regression test file to have the matching Test_suffix_test.asdf name

One question might be, why isn't the expected output Test_dark_suffix_text.asdf since the user provided --output_file="Test_dark" and --suffix="suffix_test"? This is due to dark being listed as a supported suffix.

rtdata.output = output
rtdata.get_truth(f"truth/WFI/image/{output}")
diff = compare_asdf(rtdata.output, rtdata.truth, **ignore_asdf_paths)
assert diff.identical, diff.report()


@pytest.mark.bigdata
def test_dark_current_output(rtdata, ignore_asdf_paths):
"""Function to run and compare Dark Current subtraction files. Here the
Expand Down
1 change: 1 addition & 0 deletions romancal/resample/resample_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class ResampleStep(RomanStep):
allowed_memory = float(default=None) # Fraction of memory to use for the combined image.
in_memory = boolean(default=True)
good_bits = string(default='~DO_NOT_USE+NON_SCIENCE') # The good bits to use for building the resampling mask.
suffix = string(default='resample')
""" # noqa: E501

reference_file_types = []
Expand Down
10 changes: 4 additions & 6 deletions romancal/saturation/saturation_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class SaturationStep(RomanStep):

reference_file_types = ["saturation"]

spec = """
suffix = string(default="saturation")
"""

def process(self, input):
if isinstance(input, rdm.DataModel):
input_model = input
Expand Down Expand Up @@ -46,10 +50,4 @@ def process(self, input):
ref_model.close()
sat.meta.cal_step.saturation = "COMPLETE"

if self.save_results:
try:
self.suffix = "saturation"
except AttributeError:
self["suffix"] = "saturation"

return sat
4 changes: 2 additions & 2 deletions romancal/scripts/make_regtestdata.sh
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ echo "Creating regtest files for resample..."
asn_from_list r0000101001001001001_01101_0001_WFI01_cal.asdf r0000101001001001001_01101_0002_WFI01_cal.asdf -o mosaic_asn.json --product-name mosaic
strun romancal.step.ResampleStep mosaic_asn.json --rotation=0 --output_file=mosaic.asdf
cp mosaic_asn.json $outdir/roman-pipeline/dev/WFI/image/
cp mosaic_resamplestep.asdf $outdir/roman-pipeline/dev/truth/WFI/image/
cp mosaic_resample.asdf $outdir/roman-pipeline/dev/truth/WFI/image/



Expand Down Expand Up @@ -163,7 +163,7 @@ model = AssignWcsStep.call(model)
model.to_asdf(f'${basename}_shift_cal.asdf')"
strun romancal.step.TweakRegStep ${basename}_shift_cal.asdf
cp ${basename}_shift_cal.asdf $outdir/roman-pipeline/dev/WFI/image/
cp ${basename}_shift_tweakregstep.asdf $outdir/roman-pipeline/dev/truth/WFI/image/
cp ${basename}_shift_tweakreg.asdf $outdir/roman-pipeline/dev/truth/WFI/image/
done


Expand Down
2 changes: 2 additions & 0 deletions romancal/skymatch/skymatch_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class SkyMatchStep(RomanStep):
lsigma = float(min=0.0, default=4.0) # Lower clipping limit, in sigma
usigma = float(min=0.0, default=4.0) # Upper clipping limit, in sigma
binwidth = float(min=0.0, default=0.1) # Bin width for 'mode' and 'midpt' `skystat`, in sigma

suffix = string(default="skymatch")
""" # noqa: E501

reference_file_types = []
Expand Down
1 change: 1 addition & 0 deletions romancal/tweakreg/tweakreg_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class TweakRegStep(RomanStep):
abs_nclip = integer(min=0, default=3) # Number of clipping iterations in fit when performing absolute astrometry
abs_sigma = float(min=0.0, default=3.0) # Clipping limit in sigma units when performing absolute astrometry
output_use_model = boolean(default=True) # When saving use `DataModel.meta.filename`
suffix = string(default='tweakreg')
""" # noqa: E501

reference_file_types = []
Expand Down