-
Notifications
You must be signed in to change notification settings - Fork 37
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
[WIP] Add TGLFInputs and TGLFNN #477
base: main
Are you sure you want to change the base?
Conversation
This will make it easier to integrate non-Qualikiz-based quasilinear transport models. PiperOrigin-RevId: 691455432
5c2a110
to
6a95a95
Compare
@lorenzozanisi could you lend a hand with checking the normalisations? |
/ (CONSTANTS.qe * geo.B0) ** 2 | ||
* (Ti.face_value() * CONSTANTS.keV2J) ** 1.5 | ||
/ lref | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this definition (chiGB) correct / consistent with TGLF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use quasilinear_transport_model.calculate_chiGB , where you can set a b_unit and reference length as input.
For TGLF probably b_unit=Bunit which you need to define (it's not the same as geo.B0), and aminor for the reference length. Should double check
As always, be careful with sqrt(2) in rho_s_unit and compare to TORAX calculate_chiGB to make sure we didn't miss another input argument (to include sqrt(2) or not)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I think this might've been a function added in that I hadn't noticed. I'll recheck the tglf docs for the reference field etc.
Compute gradients at rmid
@lorenzozanisi ready for your bits to be integrated as and when! |
# https://gafusion.github.io/doc/tglf/tglf_table.html#id2 | ||
Ti_over_Te = Ti.face_value() / Te.face_value() | ||
Ate = -1 / Te.face_value() * Te.face_grad(geo.rmid) | ||
Ati = -1 / Ti.face_value() * Ti.face_grad(geo.rmid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you can use the new quasilinear_transport_model.NormalizedLogarithmicGradients.from_profiles
for these. See
normalized_logarithmic_gradients = quasilinear_transport_model.NormalizedLogarithmicGradients.from_profiles( |
- I don't recommend on using the names Ate, Ati, etc., which are QuaLiKiz jargon. In any case Normalized logarithmic gradients contains general names
- Note that Ati, Ate, (in QuaLiKiz jargon) should already include the normalization length, which is aminor for TGLF. NormalizedLogarithmicGradients takes the reference length as input
/ lref | ||
) | ||
|
||
return TGLFInputs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably need alphaMHD as well? See quasilinear_transport_model.calculate_alpha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now included this as P_PRIME_LOC here. This is not treated as an independent dimension in the NN, as it can be computed from gradients, safety factor and BETAE and the NN should be able to figure this out. It does need to be used for running TGLF of course, which is where the code I linked plays a role.
Nice WIP. It is a pain to get these normalizations & miller geometry. I wonder if using the pyrokinetics would be of interest? Is there a plan to push this through in any of the next few weeks? I've got a TGLF surrogate as well, was thinking to test it in TORAX alongside ASTRA (current approach) to possibly cover model validation/comparison. I think there are also a few tglf inputs missing, namely I would be happy to contribute these. |
@fusionby2030 welcome to this PR! Glad that it might be useful to you!
Good shout - @jcitrin are you aware of pyrokinetics? I've basically been using its documentation (+Bhavin Patel, one of the developers) as my reference for trying to hook these TGLF bits in to TORAX. I feel like if there is interest in TORAX become a one-stop-shop for gyrokinetics surrogates taking advantage of the work done in pyrokinetics would be a bit of a no-brainer.
Super interested in this, keep me posted!
Yep, our surrogate doesn't use the full set of TGLF inputs (for reasons that @lorenzozanisi would be able to explain - I wasn't involved in its development!). Makes sense to extend it to the full load and make some optional or something. Please go ahead with adding whatever you need! Admin-wise, can you add to this PR given it's from a fork? Or would it need to be a branch?
@lorenzozanisi and I were tag-teaming a bit on this PR - I think Lorenzo wanted to connect full TGLF to TORAX, and has made big strides in the week before Christmas on it. Not sure where those commits are - I keep poking him to push them! Along the way, we also found a few bits that were needed to be changed more widely, such as #628 that Jonathan handled. Definitely on the agenda on UKAEA's side for January! |
To add to the pyrokinetics discussion, there are ways (albeit hacky) of passing
I have no idea how @lorenzozanisi is planning on running TGLF from TORAX without lots of IO. Would be nice if there was a python interface to TGLF... Might be best to wait until January then, am trying to coordinate a visit to Culham in February. |
I have pushed my WIP commits, sorry for not doing this earlier @theo-brown. These are not tested yet so don't attempt to use them, they are here so you can see how I am approaching the issue. @fusionby2030 Note that these commits include only the inputs specific to the NNs that I have made - so this is by no means general (and it's @fusionby2030 Note that even for QuaLiKiz not all inputs are used currently. I am not sure whether the plan is to eventually make TORAX capable of handling any number of inputs. @jcitrin how is this being handled to generalise from the 10D to the 11D case? I like the idea of including Pyro, but it sounds like TORAX is not fully IMAS compatible.
I am simply writing to the input files and running TGLF, similarly as it is done for QuaLiKiz. I shall crack on with testing my implementation now. |
fe9be98
to
1b0f261
Compare
With the open-sourcing of IMAS I would hazard a guess that IMAS compatability is moving up the priority list... |
Hi all. Thanks for this great discussion. I am aware of pyrokinetics. Garud Snoep also has gyrokit which is similar but I think he didn't open source it yet.
The QuaLiKizInputs dataclass has all that's needed for the 11D case, of which the 10D is a subset. They both have access to the same dataclass and take what they need. The plan for now is to extend these dataclasses with new inputs when necessary. e.g., we don't have a rotation profile in TORAX (easy to add when needed, although momentum transport is a task) , and when we have it we can extend to the rotation related inputs.
It may be interesting to use pyrokinetics in the future. Best would be to extend pyrokinetics itself to support "TORAX" sources of equilibrium and kinetic data. I suggest this is done after an upcoming API and variable renaming change that we are considering for the next version. Would need to consider performance and JAX compatibility though, especially with TGLF-NN. For TGLF itself this matters less since can run without JAX compilation, but there should be the same interface for TGLF and TGLF-NNs. |
Add
TGLFInputs
class andTGLFNN
model.Note: completion depends on other work in progress in #476.
Remaining tasks:
QuasilinearTransportModelInputs
QuasilinearTransportModel
physics.py
TB:
LZ:
Then check in about what integration tests are required