-
Notifications
You must be signed in to change notification settings - Fork 1
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-47068: Remove faro from pipelines. #169
Conversation
7860a7c
to
380c25d
Compare
@@ -1,5 +1,5 @@ | |||
# This BPS configuration snippet adds updated requestMemory values for all DRP | |||
# and faro tasks known to frequently exceed the default in regular ImSim-DC2 | |||
# asks known to frequently exceed the default in regular ImSim-DC2 |
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.
tasks typo
@@ -405,20 +181,6 @@ subsets: | |||
'tract' or 'patch' as part of the data ID expression if all | |||
reference catalogs or diffIm templates that cover these | |||
detector-level quanta are desired. | |||
nightlyStep5: |
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 this change necessitates that the relevant lines in the rc2_subset
repo are also removed:
https://github.com/lsst/rc2_subset/blob/1a133e3ce3b2ff950d60b48526d126463bd35fc6/bin/run_rc2_subset.sh#L119. If this isn't removed, then I'd expect any nightly test to fail when it gets to nightlyStep5
.
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.
There's also mention of nightlyStep5
inside the legacy measureHscRC2Metrics.sh
script, which also calls faro
at points. However, measureHscRC2Metrics.sh
has been superceded by run_rc2_subset.sh
, and I wouldn't object to it simply being removed at this point.
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.
Edit: please disregard the above. I clicked your PR links on the Jira ticket, but one them was duplicated and rc2_subset
was missing, so I didn't realize there was a PR for rc2_subset
as well!
I still wouldn't object if you'd rather just remove the entire measureHscRC2Metrics.sh
script however, as it should probably not be used any longer.
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 this is mostly fine, but the removal of nightlyStep5
in rc2_subset
necessitates that this step is also removed from the run_rc2_subset.sh
script inside the rc2_subset
repo.
Please disregard this comment! I did not see the rc2_subset
PR, and now that I have, I can see that you've already implemented the suggested change there!
380c25d
to
5ff4eee
Compare
5ff4eee
to
63abfc4
Compare
No description provided.