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

Flattop duration fix #773

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Flattop duration fix #773

wants to merge 3 commits into from

Conversation

daveweisberg
Copy link
Contributor

@daveweisberg daveweisberg commented Dec 3, 2024

Fixed bugs in flattop_duration matching, namely to remove the dependence on coil_j_margin.

Previously, the output flattop_duration was forced be equal to the requirements.flattop_duration * (1 + requirements.coil_j_margin)

Now the output flattop_duration is exactly equal to requirements.flattop_duration.

  • Calculation of flattop_flux_estimates should not be scaled by 1 + coil_j_margin
  • c_flt should have a margin of 0.0, not coil_j_margin
  • the margin for flattop_duration should not be dependent on coil_j_margin.

@daveweisberg daveweisberg requested a review from orso82 December 3, 2024 22:38
@orso82
Copy link
Member

orso82 commented Dec 3, 2024

@daveweisberg regression tests are failing.

Also, I am not sure I agree with the change. If one wants to have say 20% tolerance on the current limit on the ohmic solenoid, I believe that's equivalent to say that they want to have 20% tolerance on their flattop duration. In other words, the nominal flattop duration is the one cited at 20% below the absolute maximum, no?

@daveweisberg
Copy link
Contributor Author

@orso82 Ok, re-running tests with fix.

This change is important, because the previous way of doing this didn't match the main use case. What I want is a FUSE solution that has the requested flattop_duration when the CS coil current density is limited to the coil_j_margin. In practice, the CS coil would never be operated higher than this limit. So the actual flattop is never going to be longer.

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