-
Notifications
You must be signed in to change notification settings - Fork 33
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 EQDSK geometry coupling and add a new ITER hybrid test case. #482
base: main
Are you sure you want to change the base?
Conversation
First look has exposed a number of issues:
Indeed this TODO is important. For this EQDSK, epsilon=1e-7 is too small and no vertices are found (vertices is empty) for the first iteration of for loop, and vertices[0] crashes. I think it may be better to special-case the first "surface" on the magnetic axis with the expected geometry Intermediates for the magnetic axis, and then start the iteration at a position clearly not on-axis. I can also imagine there being issues with vertices if the number of surfaces requested is much higher than what the eqdsk file resolution can support. Therefore, perhaps the number of surfaces can be a geometry config input option, and can even imagine something fancy like trying to find the vertices of the first non-axis point and if it values, to reduce iteratively the number of requested surfaces until success (and output a user warning). By hacking epsilon, got this to work. Unfortunately geometry outputs looked weird, i.e. volumes and areas much less than expected for ITER. Probably not a big deal but haven't looked deeper due to lack of time. Will pick this up again later. If someone else would like a crack in the meantime please go ahead, and can push to this PR. |
IMO this is ok, as it's on the user to provide a valid EQDSK file. Were you flagging this as a problem or as a general thing to note? |
Yes, got the same error (regarding |
I agree. It was just to note. |
Another small thing I noticed that should be fixed is that Rmaj for the geometry Intermediates is taken as the axis R, as opposed to the geometric R. |
I have a pile of TSVV and WPTE tasks to finish before end of year, so it is unclear how quickly I can help out here. Although, I am interested in this, and will look into generating equilibria from CHEASE and HELENA (both output EQDSK as well) for a size & current scan to investigate the systematic differences. Targeting JET/ASDEX/TCV. But other commitments mean I can't promise anything quickly on my side. |
@theo-brown did you take a further look at this? If not I'll take a look tomorrow (Fri) |
Thanks and no problem @fusionby2030 ! |
8c8c787
to
0679700
Compare
I haven't had a chance to, sorry, and realistically I won't be able to for another few days. Feel free to have a crack at it in the meantime! |
No problem. I took a look now. Major issues
Minor issues
Will do this and update. |
0679700
to
68bcc59
Compare
New ITERhybrid EQDSK file generated from CHEASE data. Good comparison with the simulation using CHEASE geometry. Summary of fixes: 1. vpr definition bug leading to major impact on plasma volume 2. Avoiding LCFS flux-surface-integral issues for diverted cases by taking the last contour just inside the LCFS. 3. Rmaj taken now as Rgeo (LCFS Rmaj), and local Rmaj and Rmin used for delta (triangularity) calculations 4. StandardGeometryIntermediate values at the magnetic axis are prescribed, since a contour cannot be defined there. The number of surfaces to calculate, and the location of the LCFS contour with respect to the boundary psi, are user-inputs, with reasonable defaults set but need to be checked against more varied sets of EQDSK files. The EQDSK converter has only been tested against CHEASE-generated EQDSK which is at a specific COCOS. Therefore issues may arise if the input EQDSK is in a different COCOS. In near-future work we should demand that the EQDSK COCOS is an input, and we use the eqdsk convertor to convert to the TORAX COCOS, which itself also needs to be made fully self-consistent in a future PR. This important caveat is mentioned in the documentation, as well as a user-warning when running TORAX with EQDSK geometry. PiperOrigin-RevId: 691783616
68bcc59
to
a900b68
Compare
Fix EQDSK geometry coupling and add a new ITER hybrid test case.
New ITERhybrid EQDSK file generated from CHEASE data. Good comparison with the
simulation using CHEASE geometry.
Summary of fixes:
The number of surfaces to calculate, and the location of the LCFS contour with respect to the boundary psi, are user-inputs, with reasonable defaults set but need to be checked against more varied sets of EQDSK files.
The EQDSK converter has only been tested against CHEASE-generated EQDSK which is at a specific COCOS. Therefore issues may arise if the input EQDSK is in a different COCOS. In near-future work we should demand that the EQDSK COCOS is an input, and we use the eqdsk convertor to convert to the TORAX COCOS, which itself also needs to be made fully self-consistent in a future PR.
This important caveat is mentioned in the documentation, as well as a user-warning when running TORAX with EQDSK geometry.