Skip to content

Conversation

@andncl
Copy link

@andncl andncl commented Dec 18, 2025

Added parameters that are controlled by the setpoints of a ParameterWithSetpoints to the new unpack_self method (v0.54.0).

Setpoints and their corresponding inferred parameters are already being registered automatically when the ParameterWithSetpoints is registered to the measurement. This contribution adds automatic unpacking of the inferred parameters during add_result.

Potential further implementation:
This PR adds automatic adding of all parameters that are in setpoint.has_control_of. Should this also be added for setpoint.is_controlled_by? If yes I am happy to add that as well.

Very keen for feedback and opinions!

@andncl andncl requested a review from a team as a code owner December 18, 2025 03:19
@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.80%. Comparing base (c45b9eb) to head (8da633b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7732   +/-   ##
=======================================
  Coverage   59.80%   59.80%           
=======================================
  Files         352      352           
  Lines       31805    31806    +1     
=======================================
+ Hits        19022    19023    +1     
  Misses      12783    12783           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andncl
Copy link
Author

andncl commented Dec 19, 2025

@microsoft-github-policy-service agree

@samantha-ho
Copy link
Contributor

Hi Andreas,

Thank you for this contribution. I had also discovered this lack of capability and had been thinking about ways to approach the problem.

After looking at your code, I have a few thoughts:

  • The first note I have is that it seems like this code changes the expected shape of the output dataset depending on whether or not the setpoints has a has_control_of relationship or not. I consider this unwanted behavior, as ideally the dataset shape should not care if there are additional, parallel axes of data being saved.
  • Second, I note that this new code does not handle more than a single layer of has_control_of indirection. What is here is still better than no levels of indirection, but one of the use-cases I have been pondering could have multiple, nested levesl.
  • Finally, I am having a little trouble following the broadcasting logic in the new unpack_self call. It seems like each has_control_of parameter gets broadcast against the other original setpoints. It's not obvious to me that this is an intuitive way to store the data. On the other hand, the most intuitive (and performant) way to store the mappings is to not broadcast at all and simply store the individual 1D mappings (or as low-dimensional reduction as possible) directly. For readability, I'm also thinking it may be worth splitting the broadcasting/has_control_of logic into its own private method (with suitable docstring).

This last point is pretty sticky, and it's where I've struggled the most. In a high-dimensional space, or one in which one axis is significantly larger than the others (eg. where one axis is time), we could end up creating a large number of matrices to save to the dataset that are highly redundant.

Curious to hear your thoughts on these issues.

Thanks again!

  • Samantha

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