Skip to content

Injecting distortion coefficients into the method#740

Merged
dkazanc merged 1 commit into
mainfrom
distortion_coef_inject
Jul 16, 2026
Merged

Injecting distortion coefficients into the method#740
dkazanc merged 1 commit into
mainfrom
distortion_coef_inject

Conversation

@dkazanc

@dkazanc dkazanc commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Fixes issue, needs merging with this PR

image

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation
  • I have added the user-release-note label in order to include this PR in the "Notable
    Changes for Users" section in release notes

@dkazanc
dkazanc requested a review from yousefmoazzam July 16, 2026 13:25
@dkazanc dkazanc added the user-release-note Label for a PR to be included in the "user release notes" section of release notes label Jul 16, 2026
@dkazanc dkazanc changed the title Injecting distortion cofficients into the method Injecting distortion coefficients into the method Jul 16, 2026

@yousefmoazzam yousefmoazzam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One question regarding what users would want, I'm unsure.

In the case of providing a file with the distortion coefficients, I see that the file is no longer copied into the output dir. It looks like the python dict containing the params is modified which makes sense, but the YAML copied to the output dir won't have these injected coefficients values I think. If so, it looks like there's no record in the output dir of the coefficient values used for the run if a file with the coefficient values is provided, is that correct? And if so, is that OK for users?

@yousefmoazzam

Copy link
Copy Markdown
Collaborator

Ah, the screenshots from the original post in the PR shows the copied YAML does have the injected coefficient values, I must have just not seen it in the code! 😅 Apologies for being silly, but where is that happening?

@yousefmoazzam

Copy link
Copy Markdown
Collaborator

Never mind, I'm being very slow today it seems: the edited python dict is returned, and that then is copied to the output dir. Excuse my silliness... 🤣

@dkazanc

dkazanc commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

the values extracted, inserted into the method and then copied into the pipeline_conf which will be later used to dump YAML file in here. So we do not copy the original file, but the coefficients will be saved for users.
The only special case is the sweep, where injection doesn't happen, so we're not copying the file, nor saving the coefficients. Which is probably OK for users, as those are sweep runs?

@dkazanc
dkazanc merged commit 7b64a11 into main Jul 16, 2026
7 of 8 checks passed
@dkazanc
dkazanc deleted the distortion_coef_inject branch July 16, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

user-release-note Label for a PR to be included in the "user release notes" section of release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants