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

rcal-936 Update skycell_asn docs and add skycell_asn as a script at install time #1471

Merged
merged 6 commits into from
Oct 23, 2024

Conversation

ddavis-stsci
Copy link
Collaborator

Resolves RCAL-936

Closes #1458

This PR updates the read the docs to add information about the skycell_asn script to generate associations based on sky cells in the patch table from INS.

The also updates the pyproject.toml file to install skycell_asn as script.

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. 24Q4_B15 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • [N/A] update or add relevant tests
    • [N/A] update relevant docstrings and / or docs/ page
    • [N/A] start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly
news fragment change types...
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change
  • changes/<PR#>.docs.rst
  • changes/<PR#>.stpipe.rst
  • changes/<PR#>.associations.rst
  • changes/<PR#>.scripts.rst
  • changes/<PR#>.mosaic_pipeline.rst
  • changes/<PR#>.patch_match.rst

steps

  • changes/<PR#>.dq_init.rst
  • changes/<PR#>.saturation.rst
  • changes/<PR#>.refpix.rst
  • changes/<PR#>.linearity.rst
  • changes/<PR#>.dark_current.rst
  • changes/<PR#>.jump_detection.rst
  • changes/<PR#>.ramp_fitting.rst
  • changes/<PR#>.assign_wcs.rst
  • changes/<PR#>.flatfield.rst
  • changes/<PR#>.photom.rst
  • changes/<PR#>.flux.rst
  • changes/<PR#>.source_detection.rst
  • changes/<PR#>.tweakreg.rst
  • changes/<PR#>.skymatch.rst
  • changes/<PR#>.outlier_detection.rst
  • changes/<PR#>.resample.rst
  • changes/<PR#>.source_catalog.rst

@ddavis-stsci ddavis-stsci added documentation Improvements or additions to documentation associations labels Oct 23, 2024
@ddavis-stsci ddavis-stsci added this to the 25Q1_B16 milestone Oct 23, 2024
@ddavis-stsci ddavis-stsci self-assigned this Oct 23, 2024
@ddavis-stsci ddavis-stsci requested a review from a team as a code owner October 23, 2024 12:28
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Oct 23, 2024
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.15%. Comparing base (0a530f2) to head (ab63e4e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1471   +/-   ##
=======================================
  Coverage   76.15%   76.15%           
=======================================
  Files         115      115           
  Lines        7638     7638           
=======================================
  Hits         5817     5817           
  Misses       1821     1821           

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

To give a more concrete example we'll use the APT example program for the
High Latitude Wide Angle Survey (hlwas). The program is 00991, the execution plan number is 01,
there are 12 passes, 5 segments, 4 observations and a various number of visits.
For this example we want to select a single filter, say F158, and that is segment 003.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I follow this example. When I look at program 991 and pass 1 segment 3, I see a lot of exposures in different filters. Specifically, each visit is a linegap 4 dither, and those are grouped into observations of 32 pointings in a mosaic, and those observations are grouped into a segment with the F106, F129, F158, F184, and GRISM elements.

So I think the HLWAS example program has:

  • 12 passes (but note there are really only 2 passes of imaging, which are the ones we care about and the other passes are purely spectroscopic)
  • 169 segments per spectroscopic pass (basically, a big mosaic covering a lot of the sky)
  • 5 observations per segment (F106, F129, F158, F184, GRISM)
  • 32 visits per observation (a smaller mosaic covering a patch)
  • 4 exposures per visit (linegap 4 dither)
    So to select out only F158 I think you need to get observation 3.

It does look to me like it's required that all exposures in the same "observation" use the same filter, which I hadn't appreciated before.

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've updated the numbers, I was trying to simplify this for the docs. Is there another program you know of that might be clearer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The HLWAS makes sense because it's one people will have heard of. There are much smaller programs that one could use but then I just think they will be unknown to people. In general, I am not sure how well publicized the example APT programs are, but I did think it was useful to have a "worked example"; this was a reminder for me that the "execution plan" stuff is a weird detail that is pretty extraneous to our planning. It's possible we would want to ?? over that as well, but then I don't know what base name I would use, so I didn't suggest anything there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The new version still selects segment 3, but I think you need observation 3. i.e., the names need to be more like r0099101001001003001_*_cal.asdf, likewise the base names.


.. code-block:: text

skycell_asn r0099101???003*_*_cal.asdf -o r0099101001003001001 --product-type pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the right example here is more like:
skycell_asn r0099101001???003*_*_cal.asdf -o r0099101001 --product-type pass
i.e., making the path specify both the execution plan and the pass (01001) and then using the three ??? to go over all segments before specifying observation 003 to get a particular filter. Then the output root should include the activity / pass information but nothing else, and the script figures out the filter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch, I updated this from the visit and forgot to change that.

@ddavis-stsci
Copy link
Collaborator Author

ddavis-stsci commented Oct 23, 2024 via email

that this is resampled 2-d imaging data. The release product name can be changed from the default
by adding the optional argument --release-product <new name> to the command line.

An analogous command to generate the pass level products, again setting segment to 003 to only select
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
An analogous command to generate the pass level products, again setting segment to 003 to only select
An analogous command to generate the pass level products, again setting observation to 003 to only select

Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

I marked one more case of segment -> observation in line, otherwise looks good.

@ddavis-stsci ddavis-stsci merged commit e215f87 into spacetelescope:main Oct 23, 2024
31 checks passed
@ddavis-stsci ddavis-stsci deleted the rcal-936_dsd branch October 23, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
associations dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install skycell_asn as a script and update docs
2 participants