Document current Pipeline Context use cases#17
Conversation
There was a problem hiding this comment.
@sbooth-nrao Thank you for your work on combining our drafts into this document. I’ve left several comments for discussion. I have a couple of overall comments:
- There is a lot of non-use case content in this document. My suggestion is that we either trim this for brevity or move it to an appendix so the use cases are readable on their own.
- Several current use cases reference current implementation details: specific task names, class names, access patterns — at a level of specificity that makes it hard to evaluate whether RADPS needs to satisfy them. I think these should be abstracted up: written in terms of what the system needs to do, not how the current pipeline does it. These implementation details could be moved to an appendix for reference.
- For the future use cases, I think we should note whether each traces to a specific RADPS requirement, an aspect of the RADPS design that implies this use case, or is more of a ‘wish list’ item that may belong in a separate doc or section.
|
|
||
| --- | ||
|
|
||
| ## 6. Architectural Observations |
There was a problem hiding this comment.
Sections 6-8 are interesting and useful for discussing the future context design, but I don't think that this document is the best place for them. I'd prefer to keep it streamlined and focused so when we pass this document around for review and feedback about missing use cases the most relevant content is clear and isolated.
| - Role-based access to context fields | ||
| - Audit logging of all context mutations | ||
|
|
||
| ### FUC-07 — Partial Re-Execution / Targeted Stage Re-Run |
There was a problem hiding this comment.
I think this is a great idea, and I am also wondering if it is in scope? Can this be tied to a requirement for RADPS?
| - A query API (REST, gRPC, or GraphQL) | ||
| - Type definitions shared across languages | ||
|
|
||
| ### FUC-04 — Streaming / Incremental Processing |
There was a problem hiding this comment.
Can this be tied to a requirement from RADPS?
| - Artifact references rather than filesystem paths for cal tables and images | ||
| - Tasks that can operate on remote datasets without requiring local copies | ||
|
|
||
| ### FUC-03 — Multi-Language / Multi-Framework Access to Context |
There was a problem hiding this comment.
Nice to have -- is this a requirement from RADPS?
| - A merge/reconciliation step when concurrent results are accepted | ||
| - Explicit declaration of which context fields each task reads and writes | ||
|
|
||
| ### FUC-02 — Cloud / Distributed Execution Without Shared Filesystem |
There was a problem hiding this comment.
I think we could probably tie non-local execution to RADPS requirements.
There was a problem hiding this comment.
If the plan is to map these "GAP" use cases to RADPS requirements, should they even be in this document?
There was a problem hiding this comment.
This is a very good point. After reflection, I think my questions about specifically "RADPS requirements" were too narrow.
Here is my updated thought on this: I think each future use case should identify its source — whether that's an explicit RADPS requirement, something implied by the RADPS architecture, a known pain point, or something else. Without that it's hard to evaluate whether they belong here.
…ndix file; updated some wording choices for more accurate language and removed deployment-level GAP scenario
krlberry
left a comment
There was a problem hiding this comment.
I left some more comments for discussion.
| - A merge/reconciliation step when concurrent results are accepted | ||
| - Explicit declaration of which context fields each task reads and writes | ||
|
|
||
| ### FUC-02 — Cloud / Distributed Execution Without Shared Filesystem |
There was a problem hiding this comment.
This is a very good point. After reflection, I think my questions about specifically "RADPS requirements" were too narrow.
Here is my updated thought on this: I think each future use case should identify its source — whether that's an explicit RADPS requirement, something implied by the RADPS architecture, a known pain point, or something else. Without that it's hard to evaluate whether they belong here.
…t UC-06 into two use cases and update use case numbering
…date use case titles and numbering.
…n the pipeline (the Context with backticks).
…ecision making in downstream tasks. Make other assorted wording updates including removing references to removed use cases and standardizing actor names.
Use case edit suggestions
|
UC-1 metadata: also need cross-MS matching and lookup which I'd make a separate item This is one place where reinventing the wheel could be beneficial, because the current implementations use a "single master MS" and all MS were originally assumed to have the exact same sources, spw IDs, etc. |
|
Here's a PDF version of this: context_use_cases_current_pipeline.pdf |
| **Implementation notes** — the current pipeline satisfies these needs through two different propagation paths: | ||
|
|
||
| 1. **Immediate state propagation** — `Results.merge_with_context(context)` updates calibration library, image libraries, and more so later tasks can access the current processing state directly. | ||
| 2. **Serialized Results** — tasks read `context.results` to find outputs from earlier stages when those outputs are needed from the recorded results rather than from merged shared state. For example: |
There was a problem hiding this comment.
This pattern has crept in over time. The original idea was to not be dependent on results object parsing outside their native tasks. Also using explicitly the "previous" result makes assumptions about the recipe sequence. But even checking for an explicit results object type then still needs knowledge about the class structure of another task. That's why we tried using the extra attributes like "clean_list_pending" etc. Though they are, as you wrote, a bit ad-hoc and should probably at least have had a container class.
There was a problem hiding this comment.
Thanks Dirk, I will add some clarifying language regarding the intended behavior versus adapted behavior, including specific example in the code where each is used. We can also make sure to include this as an example of context creep, where an intended behavior gets lost without strict contractual definitions.
|
|
||
| **Implementation notes** — `WebLogGenerator.render(context)` in `pipeline/infrastructure/renderer/htmlrenderer.py`: | ||
|
|
||
| - Reads `context.results` — unpickled from `ResultsProxy` objects, iterated for every renderer |
There was a problem hiding this comment.
Is there really a case where all results are automatically unpickled? I thought one always had to call "read()".
There was a problem hiding this comment.
You are correct Dirk. There is a line in the htmlrenderer.py:1897 that does a mass read of all the result proxies that was mistaken as automatic unpickling of all the result objects. I will modify the language to be more representative of the actual behavior.
| - Most handlers call `context.observing_run.get_ms(vis)` to look up metadata for scoring (antenna count, channel count, SPW properties, field intents) | ||
| - Some handlers check `context.imaging_mode` to branch on VLASS-specific scoring | ||
| - Others check things in `context.observing_run`, `context.project_structure`, or the callibrary (`context.callibrary`) | ||
| - Scores are appended to `result.qa.pool`, so the scores are stored on the results rather than directly on the context. |
There was a problem hiding this comment.
I don't remember if this was due to some size consideration too. We can potentially have many QA score objects in the pool if there is one per detailed data selection (field/spw/pol/ant/baseline/...).
There was a problem hiding this comment.
Added language to the UC-15 implementation notes indicating current implementation behavior. Also made a note to create explicit rules to restrict this in future designs.
docs/context_gap_use_cases.md
Outdated
| - GAP-03: Provenance and reproducibility — requires immutable per-attempt records, input hashing, and lineage capture. | ||
| - GAP-04: Partial re-execution / targeted rerun — requires explicit dependency tracking and invalidation semantics at the context level. | ||
| - GAP-05: External system integration — requires stable identifiers, event subscriptions/webhooks, and exportable summaries/manifests. | ||
| - GAP-06: Multi-language access — requires a language-neutral schema and API for context state and artifact queries. |
There was a problem hiding this comment.
Do you mean "programming language"?
For a low barrier approach it would be good to have something like a middleware so that the local language API does not need to expose the actual structure of how the context is stored. And since we/I think that the dev team should be able to add new items quickly, some "standard" data types (incl. dictionaries or equivalent) should be readily available.
There was a problem hiding this comment.
I renamed GAP-06 to explicitly mention Programming Language and included a recommendation for a stable middleware layer.
…-12/UC-14 impl notes; add GAP-08 (cross-MS matching); update GAP-06 title and summary
tnakazato
left a comment
There was a problem hiding this comment.
@sbooth-nrao @krlberry thank you very much for your work. The document is comprehensive and is very good high-level summary of the usecase. I made a few comments. I would appreciate it if you could take a look.
…dd GAP-08, refine GAP-06)
Adds document that catalogues how the current ALMA/VLA pipeline uses its
Contextobject.context_use_cases_legacy_pipeline.md — 17 use cases (UC-01 – UC-17) describing what the current pipeline context does. The initial draft was merged from drafts by Berry and Booth.
docs/context_current_pipeline_appendix.md - an appendix which describes the implementation of the current context use cases.