Skip to content

Add scripts to run LOCO fitting & set fitted-strengths to the machine#179

Merged
anacso17 merged 19 commits intomasterfrom
loco-strengths-script
Jul 28, 2025
Merged

Add scripts to run LOCO fitting & set fitted-strengths to the machine#179
anacso17 merged 19 commits intomasterfrom
loco-strengths-script

Conversation

@vellosok75
Copy link
Copy Markdown
Contributor

@vellosok75 vellosok75 commented May 9, 2025

Currently, LOCO-fitted strengths are applied to the machine via the as_magnets_manipulate_strengths.ipynb notebook. However, this notebook includes also other functionalities and a particular workflow which increases the risk of operational mistakes. This PR introduces a dedicated script to streamline the process, reduce human error, standardize and simplify the setting up of strengths.

Partner of lnls-fac/apsuite#297, lnls-fac/apsuite#299

Also includes script to run LOCO with proper input parsing. This script has been used for years but not tracked in any repository. See the helper:

image

vellosok75 and others added 2 commits May 9, 2025 15:37
@anacso17
Copy link
Copy Markdown
Contributor

anacso17 commented Jun 4, 2025

@vellosok75 , please, update version to 0.21.0

Copy link
Copy Markdown
Contributor

@fernandohds564 fernandohds564 left a comment

Choose a reason for hiding this comment

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

are all these user interactions really necessary? I tend to prefer having optional arguments for the script, instead of having to keep responding to questions along the running time. These questions make it harder to automatize things using the script... why did you decide to go this way? This is not the standard way we create scripts.

@murilobalves
Copy link
Copy Markdown
Contributor

murilobalves commented Jun 5, 2025

are all these user interactions really necessary? I tend to prefer having optional arguments for the script, instead of having to keep responding to questions along the running time. These questions make it harder to automatize things using the script... why did you decide to go this way? This is not the standard way we create scripts.

Thanks for raising these points @fernandohds564

I initially started moving towards using a parser, but later recalled that this script is actually meant to replace a Jupyter notebook. Given that it involves applying changes to magnets, it makes sense to include some form of user confirmation. So, I see this script as a middle ground between our standard scripts and notebooks. I agree that it sacrifices some automation, but in this specific case, I believe automation is unlikely to be needed.

I ended up following the same interactive steps proposed by Matheus to the run-loco script, to allow some flexibility to the user. But while I'm writing this I realize that losing the possibility of automation for the LOCO fitting script is bad.

While I still think that for the set-loco-strengths the iterative mode would be acceptable (I would be fine in using parser as well for the sake of standards), for the run-loco-fitting I agree that using the parser is definitely the best option. Will work on that tomorrow!!

@vellosok75
Copy link
Copy Markdown
Contributor Author

are all these user interactions really necessary? I tend to prefer having optional arguments for the script, instead of having to keep responding to questions along the running time. These questions make it harder to automatize things using the script... why did you decide to go this way? This is not the standard way we create scripts.

@fernandohds564, the automation issue is a good point. This particular workflow emerged as I tried to streamline the same processes used in the notebook, but avoiding having to go back and forth between notebook cells. So, in a sense, this became a mere translation and the script ended up rather interactive.

When calling the corresponding apply_func, the changes that will be applied to the corresponding magnets are printed. We visually and loosely compare these changes to those indicated by the fitting and if they are compatible we accept them. We call the function again, and expect the required strengths printed to be very close to zero. So we have two steps: check if the strengths to be applied are compatible with the fit, and then check if they been successfully applied.

So, yeah, the script streamlines the process, but still leaves things rather manual. I had not thought of it before, but now that this issue has been raised, I think we can (and I personally prefer to) automate these verifications. What do you guys think @murilobalves @xresende?

@vellosok75 vellosok75 changed the title Script to set LOCO strengths Add scripts to run LOCO fitting & set fitted-strengths to the machine Jun 5, 2025
@fernandohds564
Copy link
Copy Markdown
Contributor

Well, maybe the use will tell us.
Regarding the set-to-machine script, I think we can start this way and change things if we notice it is too cumbersome to run. Regarding the LOCO fitting script, I think I aggree with you @murilobalves. It is better to let it run freely, handling the options with the script's inputs.

@murilobalves
Copy link
Copy Markdown
Contributor

@fernandohds564 @vellosok75 I changed run-loco-fitting to use argparse, please take a look

Copy link
Copy Markdown
Contributor

@fernandohds564 fernandohds564 left a comment

Choose a reason for hiding this comment

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

the fitting script seems ok, but there still are two comments open in the apply-to-machine script.

@vellosok75
Copy link
Copy Markdown
Contributor Author

the fitting script seems ok, but there still are two comments open in the apply-to-machine script.

Yes. Couldn't work on them today but will do monday morning.

@vellosok75 vellosok75 requested a review from fernandohds564 June 9, 2025 20:32
@xresende
Copy link
Copy Markdown
Contributor

arriving late at the discussion... the advantages of a non-interactive script are bigger but I think it is usefull if the script prints back all options used in the fitting: it helps leaving the "auto mode" every now and then when we run it.

@vellosok75
Copy link
Copy Markdown
Contributor Author

Guys, I changed the script and the base class methods for allowing step-by-step setting of the families strengths in percentages.

As you may remember, the main difficulty was that the set_delta_strength methods would always calculate the required change to apply based on the current state of the machine. I now added the option to use a fixed initial reference.

What do you think? @murilobalves @fernandohds564 @xresende

@anacso17
Copy link
Copy Markdown
Contributor

anacso17 commented Jul 8, 2025

Hi @vellosok75 , do you want to include this one in the next deploy? If yes, can you please update package version to 0.21.0?

@fernandohds564
Copy link
Copy Markdown
Contributor

@anacso17 , @vellosok75 in on vacation. I think we'll leave this one for a next opportunity.

@anacso17 anacso17 merged commit 33746c7 into master Jul 28, 2025
@anacso17 anacso17 deleted the loco-strengths-script branch July 28, 2025 15:04
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.

6 participants