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

Add field to things for boundary error objectives #935

Merged
merged 9 commits into from
Mar 19, 2024
Merged

Conversation

f0uriest
Copy link
Member

@f0uriest f0uriest commented Mar 7, 2024

  • Adds the field parameters as optional inputs to {Vacuum}BoundaryError.compute
  • Adds field_fixed option to objectives - currently this doesn't do anything since we usually can't precompute field qtys for a changing boundary, but could add special logic for certain field types later. Mainly this flag is for use in regular free boundary solve so you don't need to include explicit constraints targeting the field.

Resolves #855

Should ideally merge #726 and #853 first so we have some sensible coil constraints before doing single stage stuff

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@f0uriest f0uriest marked this pull request as draft March 7, 2024 22:43
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

Attention: Patch coverage is 96.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 94.63%. Comparing base (073efec) to head (6797100).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
- Coverage   94.64%   94.63%   -0.02%     
==========================================
  Files          86       86              
  Lines       21580    21586       +6     
==========================================
+ Hits        20424    20427       +3     
- Misses       1156     1159       +3     
Files Coverage Δ
desc/magnetic_fields/_core.py 82.35% <100.00%> (-0.31%) ⬇️
desc/objectives/_free_boundary.py 96.87% <100.00%> (+0.06%) ⬆️
desc/io/equilibrium_io.py 86.22% <92.30%> (-0.96%) ⬇️

@f0uriest f0uriest marked this pull request as ready for review March 16, 2024 05:58
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why were these changes necessary for this PR?

Copy link
Member Author

@f0uriest f0uriest Mar 17, 2024

Choose a reason for hiding this comment

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

SplineMagneticField requires some custom behavior to work correctly as a pytree (required for it to be part of things), and the previous method of defining a custom flattening method didn't consider all the extra attributes that get added with the Optimizable base class. This refactors things a bit so that each class can use the default flattening method but also define specific attributes as static or dynamic without having to re-define the entire flattening/unflattening method (and then having to account for every attribute individually)

@f0uriest
Copy link
Member Author

Note benchmarks are expected to fail due to updated argument name (renamed ext_field -> field)

@f0uriest f0uriest merged commit 4eb9b79 into master Mar 19, 2024
18 of 31 checks passed
@f0uriest f0uriest deleted the rc/freeb_field branch March 19, 2024 21:53
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.

Add field to things in free boundary objective to allow single-stage free-boundary calculations
4 participants