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
DM-37984: Move rc2_subset DRP pipeline definitions to drp_pipe #54
Conversation
39189a5
to
bcf7001
Compare
20760be
to
f17fad2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small comments, but mostly looks good. Note that we'll also need to update the shell script in rc2_subset that executes the processing.
pipelines/HSC/DRP-RC2_subset.yaml
Outdated
time, because otherwise quanta will be created for jobs with only partial | ||
visit coverage. | ||
isolatedStarAssociation can be run with or without 'tract' constraints. | ||
nightlyStep2cde: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work to include FGCM in the same step with the other tasks? In rc2_subset, FGCM has been separate because it explicitly has to be run on a single processor. Is that true of the other tasks here, or would it improve performance to keep them separate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the shell script that executes rc2_subset processing at https://github.com/lsst/rc2_subset/blob/bf14286d29ed909cd0b0949231dc2b19725cb8d1/bin/measureHscRC2Metrics.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jeff. Following a conversation with @erykoff, I've decided to revert this change and maintain the three FGCM tasks in their own nightlyStep3c
subset, with the remainder continuing on as before in nightlyStep2d
. To that end, the only actual pipeline changes should now be the addition of makeVisitTable
and makeCcdVisitTable
into nightlyStep2d
. Other proposed changes, such as moving isolatedStarAssociation
from nightlyStep2b
into nightlyStep2a
(as in DRP-RC2.yaml
) have also been dropped.
visit coverage. | ||
isolatedStarAssociation can be run with or without 'tract' constraints. | ||
nightlyStep2cde: | ||
subset: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think TE3 and TE4 should also be in this subset (per-visit tasks)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have been moved back to their former home in nightlyStep2d
.
pipelines/HSC/DRP-RC2_subset.yaml
Outdated
- TE3 | ||
- TE4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See note above - TE3 and TE4 are per-visit tasks, so should be in Step2cde?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have been moved back to their former home in nightlyStep2d
.
pipelines/HSC/DRP-RC2_subset.yaml
Outdated
detector-level quanta are desired. | ||
nightlyStep5: | ||
subset: | ||
- nsrcMeasVisit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a per-visit task, so should be in Step2cde?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been moved back to its former home in nightlyStep2d
.
f17fad2
to
772af6a
Compare
No description provided.