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

Fix: py_spec handling for mvol=1 #204

Merged
merged 6 commits into from
Nov 14, 2024

Conversation

missing-user
Copy link
Contributor

@missing-user missing-user commented Nov 12, 2024

This PR handles some edge cases I encountered in the py_spec Utilities/pythontools/py_spec/output module and plotting library, specifically surrounding equilibria with nvol=1. In that case, quantities like pressure return a scalar instead of a 1 element array, which isn't correctly handled in the plotting methods.

  • Handle plotting for equilibria with mvol=1
  • Consistent handling of lsurf parameter (some methods accepted scalars and numpy arrays, some threw exceptions when out of range, some didn't... ). Now all methods have the same error handling for lsurf.
  • Fixed type annotations to methods such as get_surface_current_density, because lsurf expected a numpy array and not an integer!

@missing-user missing-user changed the title Fix: py_spec Fix: py_spec handling for mvol=1 Nov 12, 2024
@smiet smiet self-requested a review November 14, 2024 09:25
Copy link
Collaborator

@smiet smiet left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution, it is highly appreciated!

The python wrappers could indeed use some work, and I am very happy to help with any future changes you would want or need.

@smiet smiet merged commit 6d2693c into PrincetonUniversity:master Nov 14, 2024
5 checks passed
@missing-user
Copy link
Contributor Author

Thanks a lot! I'm doing some stellerator optimization with SPEC and SIMSOPT, and will be fixing the issues I encounter as I go along.

I do have one potential bug, that I'm pretty confused about at the moment, but I'll open a separate issue for that.

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