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

DM-48122: Remove ProcessCcd.yaml from ap_pipe #218

Merged
merged 6 commits into from
Jan 14, 2025
Merged

Conversation

kfindeisen
Copy link
Member

This PR removes ProcessCcd.yaml and RunIsrWithCrosstalk.yaml, moving their contents into the main pipelines and a config file, respectively. This change removes all pipeline splicing from ap_pipe (though we still have some in ap_verify).

With the removal of ProcessCcd.yaml, this PR also deprecates the processCcd subset, replacing it with apPipeSingleFrame.

Copy link
Collaborator

@mrawls mrawls left a comment

Choose a reason for hiding this comment

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

DECam is so much nicer now! As far as I can tell, this looks good, minor comments notwithstanding, and I appreciate you running the CI data sets to verify (ha) they all still work.

The two single-frame tasks (ISR and CalibrateImage) are more naturally
defined in the AP pipeline itself. However, the processCcd subset is
still needed for backward-compatibility. Including
RunIsrWithCrosstalk.yaml still requires splicing, but one fewer level
of it.
Now that ProcessCcd is no longer used by higher-level pipelines, it can
be removed without consequences.
This change removes the last of the splicing, and the subset
redefinition that requires. Code duplication in the ISR config itself
is avoided by moving it to a config file.
In general, we want to run single-frame metrics with single-frame
processing. Previously, this was impractical because the tasks and
subsets were defined in different places.
The name "processCcd" is a throwback to the Gen 2 ProcessCcdTask, and
is not intuitive for new users who've only known Gen 3. The replacement
name apPipeSingleFrame conveys the term "single-frame processing"
(which is still in use) while distinguishing between the subset of
ApPipe and the standalone SingleFrame pipeline (which are conceptually
different enough that they must not be mixed up).
@kfindeisen kfindeisen merged commit 98da283 into main Jan 14, 2025
2 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-48122 branch January 14, 2025 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants