Skip to content

Conversation

@hruth
Copy link
Collaborator

@hruth hruth commented Dec 11, 2024

  • Removed OPR mode update methods from LSQMLReconstructor and added them into OPRModeWeights
  • Removed OPR mode settings from LSQML options classes and added them to the base options classes
  • Added OPR mode support for PIE type reconstructors

@hruth hruth requested a review from mdw771 December 11, 2024 16:27
@mdw771
Copy link
Collaborator

mdw771 commented Dec 11, 2024

Can you also add Ash as a reviewer?

@hruth
Copy link
Collaborator Author

hruth commented Dec 11, 2024

I also found a bug in update_opr_probe_modes_and_weights .

probe[i_opr_mode, 0, :, :] = eigenmode_i updates reconstructor.parameter_group.probe.data since probe is a reference to reconstructor.parameter_group.probe.data. It seems like this is unintentional. The same is true for the weights. You should expect that the fix to this will change the algorithms results, but only for the case where probe optimization is on and weight optimization is off or vice-versa. I'll fix this issue in the next commit.

@hruth hruth requested review from a4894z and removed request for mdw771 December 11, 2024 16:51
@hruth hruth closed this Dec 11, 2024
@hruth hruth reopened this Dec 11, 2024
@hruth
Copy link
Collaborator Author

hruth commented Dec 11, 2024

Can you also add Ash as a reviewer?

I just did. Annoyingly, GitHub only allows a single reviewer at a time if you are using the free version. I suppose it's good enough to just add reviewers so that they get an email notification about it, even if they don't keep the reviewer status on the PR.

@mdw771
Copy link
Collaborator

mdw771 commented Dec 11, 2024

I also found a bug in update_opr_probe_modes_and_weights .

probe[i_opr_mode, 0, :, :] = eigenmode_i updates reconstructor.parameter_group.probe.data since probe is a reference to reconstructor.parameter_group.probe.data. It seems like this is unintentional. The same is true for the weights. You should expect that the fix to this will change the algorithms results, but only for the case where probe optimization is on and weight optimization is off or vice-versa. I'll fix this issue in the next commit.

self.parameter_group.probe.data returns a copy. See ReconstructParameter.data(). The updated copies are set back to the probe and OPR weight objects in the subsequent lines.

@hruth hruth closed this Dec 19, 2024
@mdw771 mdw771 deleted the pie_opr_modes branch July 25, 2025 18:34
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.

3 participants