Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMI430: Restore ramp rates of individual magnet axes after simultaneous ramp, for block_during_ramp is True or with call to wait_while_all_axes_ramping #3885

Merged
merged 23 commits into from
Feb 16, 2022

Conversation

astafan8
Copy link
Contributor

@astafan8 astafan8 commented Feb 1, 2022

Apparently, it is confusing for users that the simultaneous ramp mode does not keep the ramp rate settings of the individual magnet axes intact. User's impression of the simultaneous ramp mode with the vector_ramp_rate parameter is that it sits on top of the individual axes and after the ramp is over, the individual magnet axes should still have their original individual ramp rates preserved.

In the implementation, restore_at_exit context managers of the ramp_rate parameters are used. The get exit-ed aka closed in the wait_while_all_axes_ramping, which means that not only users can run the simultanous ramps with block_during_ramp is True and the restoring will happen, but also the restoring will happen when users call wait_while_all_axes_ramping explicitly, even when block_during_ramp is set to False.

  • test on real instrument
  • test if after initiating a ramp, it is possible to change ramp rate on the fly; does instrument allow it or errors out, does it change the ramp rate or ignores it, etc

Credits to @JouriB for the idea and the help with testing, and @jenshnielsen for refining the implementation!

@astafan8 astafan8 added the driver label Feb 1, 2022
@astafan8 astafan8 added this to the 0.33.0 milestone Feb 1, 2022
@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #3885 (840ddfe) into master (b3a7e38) will increase coverage by 0.27%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3885      +/-   ##
==========================================
+ Coverage   65.80%   66.07%   +0.27%     
==========================================
  Files         228      228              
  Lines       31129    31147      +18     
==========================================
+ Hits        20483    20579      +96     
+ Misses      10646    10568      -78     

@astafan8 astafan8 marked this pull request as ready for review February 1, 2022 22:26
@JouriB
Copy link

JouriB commented Feb 4, 2022

I tested the PR, it works as expected.

I also tested what happens when the ramp rate is changed during a ramp started with blocking. The ramp rate is in fact changed on the fly for the active ramp. So restoring the ramp rates for unblocked ramps does not seem to be possible.

image

@astafan8 astafan8 changed the title AMI430: Restore ramp rates of individual magnet axes after simultaneous ramp but only if block_during_ramp is True AMI430: Restore ramp rates of individual magnet axes after simultaneous ramp, for block_during_ramp is True or with call to wait_while_all_axes_ramping Feb 15, 2022
@astafan8 astafan8 merged commit 821844e into master Feb 16, 2022
@bors bors bot deleted the astafan8/ami-ramp branch February 16, 2022 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants