Refactor iota computation and add limit #448
Conversation
unalmis
commented
Mar 8, 2023
•
unalmis
commented
- Refactor the computation of iota and add ability to compute iota at the rho=0 axis
- Computation shows the limit appears to be a continuous extension of iota to the axis. As long as it actually is, I think this is sufficient to formally justify that interchange of the integral and limit we discussed. my_math.pdf
- Correct QAS input file
- The input file on master for QAS was generated sometime between September (I think) and October 15. The one I uploaded in thie PR is the one generated by DESC on October 15 (and could be regenerated when this PR was made) that had better agreement with VMEC's QAS solve. Although, the VMEC input input.QAS.txt to DESC input converter recently (as in sometime close to June 6 2023) gives something different now.
- The reason this change is included in this PR is because one test on QAS for the iota limit computation includes a comparison to a hardcoded value of iota that comes from solving the QAS input file. So if I want that test to be more accurate I should make sure QAS is being solved well.
- Implement efficient API for limits.pdf
- Skipped part 3 for now due to Rory's suggestion; although the broadcasting is not an issue (updated the pdf to show why)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #448 +/- ##
===========================================
+ Coverage 67.76% 94.01% +26.25%
===========================================
Files 73 73
Lines 16452 16497 +45
===========================================
+ Hits 11148 15510 +4362
+ Misses 5304 987 -4317
☔ View full report in Codecov by Sentry. |
unalmis
added
low priority
|
@unalmis what is the status on this? I am starting to write my paper where I use on-axis quantitites from DESC as a verification against near-axis theory, and it could be nice to be able to just say "on-axis values from DESC" and not have to sweep under the rug the fact that we are calculating things at |
Would you like to just compute iota and its derivatives at 0, or are you asking for the status of #486? If its the former, I can probably do the math for the |
| @@ -528,7 +534,9 @@ def build(self, eq, use_jit=True, verbose=1): | |||
|
|
|||
| self._dim_f = surface_grid.num_nodes | |||
| self._data_keys = ["R", "phi", "Z"] | |||
| self._args = get_params(self._data_keys) | |||
| self._args = get_params( | |||
| self._data_keys, has_axis=plasma_grid.axis.size or surface_grid.axis.size | |||
There was a problem hiding this comment.
FYI the surface grid should never have a point at the axis (since its a 2d grid over a surface) and realistically the plasma grid shouldn't either.
| """ | ||
| deps = { | ||
| "params": params, | ||
| "transforms": transforms, | ||
| "profiles": profiles, | ||
| "data": data, | ||
| "axis_limit_data": [] if axis_limit_data is None else axis_limit_data, |
There was a problem hiding this comment.
why not have the default be axis_limit_data=[] instead of None, and then this line can simply be "axis_limit_data": axis_limit_data (like all the other fields)
There was a problem hiding this comment.
Using a mutable object as a default parameter can lead to bugs. Because the default parameter of the function stores a reference to some list, if that list is modified, then the default value of the function changes accordingly to the modified list.
We don't plan to ever modify a quantity's axis_limit_data list, so you're right that we can use the same empty list for axis_limit_data for all quantities that don't specify it in the compute_funs.
We use this same None to new empty list design pattern in other places of the code. Let me know if you still want me to change
|
I'm going to merge this since the changes since Dario's approval were cosmetic |
