Skip to content

Return starting model and config file for delay calibration#101

Merged
tikk3r merged 7 commits intomasterfrom
return-delay-config
Mar 27, 2026
Merged

Return starting model and config file for delay calibration#101
tikk3r merged 7 commits intomasterfrom
return-delay-config

Conversation

@tikk3r
Copy link
Copy Markdown
Member

@tikk3r tikk3r commented Mar 9, 2026

I recently found myself in multiple scenarios where I needed to re-run or tweak the delay calibration on difficult fields. With settings and skymodels now being generated automatically, this is cumbersome at the moment as they are not outputs of the main workflow and as such do not "survive" a successful pipeline run. This PR adds the starting models as outputs of the pipeline as well as the config file that comes from phaseup-concat (the automatic selection does not yet generate config files, I'll leave that for a future PR). This should improve reproducibility in general as well by having easier access to the combination of model and settings that were used.

@tikk3r tikk3r requested a review from lonbar March 17, 2026 10:17
Copy link
Copy Markdown
Member

@lonbar lonbar left a comment

Choose a reason for hiding this comment

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

Looks good to me. A few minor syntax nits.

Comment thread workflows/subworkflows/find-best-delay-calibrator.cwl Outdated
Comment thread workflows/delay-calibration.cwl
@lonbar
Copy link
Copy Markdown
Member

lonbar commented Mar 17, 2026

I'm wondering if it makes sense to bundle outputs such as these into a record or perhaps a JSON file. It seems more like metadata; the user is not interested in these output per se. It is definitely a good idea to store them, but bundling it up could perhaps simplify the outputs that we have in the workflows. What do you think?

@tikk3r
Copy link
Copy Markdown
Member Author

tikk3r commented Mar 25, 2026

I'm wondering if it makes sense to bundle outputs such as these into a record or perhaps a JSON file. It seems more like metadata; the user is not interested in these output per se. It is definitely a good idea to store them, but bundling it up could perhaps simplify the outputs that we have in the workflows. What do you think?

I'm not sure I fully understand. Do you mean in the sense of the contents of these files in a JSON file?

@lonbar
Copy link
Copy Markdown
Member

lonbar commented Mar 26, 2026

@tikk3r perhaps, yes. I am mostly thinking out loud here. We could collect and store all the individual files, but perhaps it's useful to create a data structure that contains all the parameters and their values per parset, perhaps with some information/metadata such as which steps they are used in.

@tikk3r
Copy link
Copy Markdown
Member Author

tikk3r commented Mar 26, 2026

@tikk3r perhaps, yes. I am mostly thinking out loud here. We could collect and store all the individual files, but perhaps it's useful to create a data structure that contains all the parameters and their values per parset, perhaps with some information/metadata such as which steps they are used in.

It sounds like an interesting idea. The nice thing about having the files is that they are in the format expected by the software, so we don't have to reconstruct it. Maybe something for another time?

@tikk3r tikk3r force-pushed the return-delay-config branch from 44643d6 to 382fe9f Compare March 26, 2026 20:22
@lonbar
Copy link
Copy Markdown
Member

lonbar commented Mar 27, 2026

It sounds like an interesting idea. The nice thing about having the files is that they are in the format expected by the software, so we don't have to reconstruct it. Maybe something for another time?

That's fine. My main concern was that it may not be obvious which file to apply where, but we can leave it as it is for now.

@tikk3r tikk3r merged commit bb1853d into master Mar 27, 2026
8 checks passed
@tikk3r tikk3r deleted the return-delay-config branch March 27, 2026 14:43
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