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

The E coils and F coils do not use same unit #267

Closed
wants to merge 1 commit into from

Conversation

AreWeDreaming
Copy link
Collaborator

The E and F coils are combined when loading from the EFIT tree. However, the F coils have the turns multiplied with the current, which one could argue is an preexisting error in MDS+. This would be at least consistently wrong but the F-coils are combined with the E-coils in pf_current in constraints which are stored correctly.
Following options:

  • Merge this and remove the mapping completely
  • Try to fix the mapping, but this is tricky because the information on how many windings each coil has is not MDS+. For DIII-D there is some expectation that the F-coils always have the same amount of turns for all shots, but the EFIT tree and mapping is not DIII-D specific, so it gets complicated very quickly.
  • Leave it because it is consistent how the code wants its pf_currents. Here it is important to remember that this is the mapping for EFIT not the mapping for pf_active.
    I leave it to @smithsp to make an executive decision and @bechtt to add his own opinion.

@torrinba
Copy link
Collaborator

The second option would be the best in my opinion. The MDS+ organization used by different machines (if they even do) is not universal, so I would consider most of the EFIT mapping to be DIII-D specific. Some parts are the same for NSTX (and are used), but I would consider that an exception rather than the norm.

I also think leaving pf_current with different units/interpretations for different elements is confusing for any use case besides EFIT and does not follow the IMAS definition (pf_current specifies it should have the same size as pf_active.coil and pf_active.coil[:].current is defined as the measured current in a single turn).

My second choice would be the first option, which should be fine temporarily since this isn't being used anywhere yet.

@github-actions
Copy link

Stale pull request message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants