Skip to content

Conversation

@theo-brown
Copy link
Collaborator

@theo-brown theo-brown commented Sep 1, 2025

  • Add conversions j_tor <-> j_parallel
    • Unit tests
    • Comparison against JETTO
    • Fix on-axis conversion errors
  • Change total_psi_sources
  • Change individual psi sources:
    • ECRH
    • Generic current source
  • Add j_tor and j_parallel to output
  • track down everywhere j is assumed toroidal
    • Check j_ohmic
    • Check initialisation
    • integrated currents
    • sawteeth
  • Regenerate tests

#1459

@theo-brown theo-brown marked this pull request as draft September 1, 2025 22:48
@theo-brown
Copy link
Collaborator Author

theo-brown commented Sep 2, 2025

@jcitrin am I right that the noninductive psi sources are ECCD, bootstrap, and generic current source? Am I missing any?

ECCD is field-aligned following #1363. Bootstrap is field aligned by definition?

Currently the generic current source is set to provide a fixed toroidal current in A (or fraction of the total current). Would you like this to remain as specifying a toroidal current, or is it ok to silently change it to being the specification of a field-aligned current? Can add to the generic current source either documentation saying it's a <j.B> current or add the conversion from j_tor to <j.B>.

@theo-brown theo-brown changed the title Switch psi sources to being <j.B> Switch psi sources to being <j.B>/B_0 Sep 3, 2025
@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 3 times, most recently from 5275110 to da2de19 Compare September 3, 2025 10:13
@theo-brown theo-brown marked this pull request as ready for review September 3, 2025 10:27
@theo-brown theo-brown requested a review from jcitrin September 3, 2025 10:27
@theo-brown theo-brown linked an issue Sep 4, 2025 that may be closed by this pull request
@jcitrin
Copy link
Collaborator

jcitrin commented Sep 4, 2025

@jcitrin am I right that the noninductive psi sources are ECCD, bootstrap, and generic current source? Am I missing any?

For now yes. In principle there could be NBI or Lower Hybrid (LH) current sources, but we don't have models for them yet.

Bootstrap is field aligned by definition?

Yes, the output of the bootstrap model is <j \cdot B>/B_0 . So its usage in the psi equation is correct. However, what is incorrect is our interpretation elsewhere that it is flux-surface-averaged toroidal current. For example in the output I_p, I_bootstrap, I_ecrh, everything is assumed as toroidal current. In any internal calculations calculating j_ohmic etc., for the initialization stuff, then we assume that all currents are always toroidal.

What we need are conversion formulas from <j \cdot B>/B_0 to j_torax = <j_phi / R> / <1/R> , and vice versa. I'll make a separate issue for this (even though it's intimately linked to this issue).

Currently the generic current source is set to provide a fixed toroidal current in A (or fraction of the total current). Would you like this to remain as specifying a toroidal current, or is it ok to silently change it to being the specification of a field-aligned current? Can add to the generic current source either documentation saying it's a <j.B> current or add the conversion from j_tor to <j.B>.

I think the generic current source input description should remain "toroidal current" which is more user intuitive. Then internally we have the conversions to what's need to the psi equation, as discussed.

Thanks for being on top of this one!

@theo-brown
Copy link
Collaborator Author

Cool, I'll implement and test the conversion in #1508 and then hunt down all the places it needs to be converted in here.

@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 2 times, most recently from bcc0d44 to 73902ae Compare September 11, 2025 19:54
@theo-brown
Copy link
Collaborator Author

theo-brown commented Sep 11, 2025

Currently, this is set up to have core_sources.psi as containing current densities in <j.B>/B0 form. This makes sense to me as it matches what the sources in the psi equation are expected to be.

My current plan is to add toroidal current densities to PostProcessedOutputs and then explicitly state in the output file which is which. Does j_toroidal_ecrh vs j_parallel_ecrh as a naming convention for the outputs sound ok?
The outputs would look like this:

image

@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 5 times, most recently from fb132ff to 271e18b Compare September 11, 2025 21:35
@theo-brown theo-brown linked an issue Sep 13, 2025 that may be closed by this pull request
@jcitrin
Copy link
Collaborator

jcitrin commented Sep 29, 2025

The convention parallel vs toroidal is good.

However, we have committed not to make breaking changes to output API until V2. This means that new additions can be made, but name changes cannot. So I recommend having something like j_foo_parallel and j_foo, where j_foo is toroidal, and this is mentioned explicitly in the docs. A TODO for name updates conversion to V2 can be made.

@theo-brown
Copy link
Collaborator Author

theo-brown commented Oct 3, 2025

Is it possible to run with one of our EQDSK or IMAS equilibrium inputs in both JETTO and TORAX, and verify a conversion "by eye", and then after doing that just put the j_parallel_truth and j_toroidal_truth (for a small grid) by hand in this file, and load that same geo? Then at least it's a regression test.

Turns out JETTO also has inconsistencies in its IMAS outputs with <j.B> vs j_toroidal, so we're fixing that upstream. Once that's sorted, we'll be able to do the comparison easily!

@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 2 times, most recently from 943559a to 81e4ce5 Compare October 31, 2025 15:14
@theo-brown theo-brown requested a review from jcitrin November 19, 2025 22:36
@theo-brown
Copy link
Collaborator Author

@jcitrin ready for internal review.
Haven't exhaustively checked all the test results make sense, just looked at a representative sample.
Can also squash to make a single commit if desired.

@jcitrin
Copy link
Collaborator

jcitrin commented Nov 20, 2025

Great stuff. Yes please squash the commits, I'll then bring it in

@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 6 times, most recently from 49b2dac to 1dc3223 Compare November 21, 2025 15:01
@theo-brown theo-brown requested a review from jcitrin November 21, 2025 15:23
@theo-brown theo-brown force-pushed the psi-sources-j-dot-B branch 5 times, most recently from 5cd2722 to 8d5b46b Compare November 27, 2025 13:42
@theo-brown
Copy link
Collaborator Author

@jcitrin thanks for the review comments :)

Current density profile plots on STEP for reference: you can see that the js now do all add up to the j_total. Bonus is that j_ohmic is now visibly near zero for most of the core, as expected, so this has knock-on improvements for simulations with vloop BC.

On-axis behaviour should be investigated in a follow-up PR.

image

- Add conversion functions to psi_calculations
 - Compute derivatives manually from face grid values for improved accuracy
 - Manually extrapolate on-axis values to avoid numerical artifacts
 - Tested on STEP geometry for grid sizes 25-1000
 - Unit test
- In outputs: change j_* to be toroidal, and add j_parallel_*
- Bootstrap: rename to j_parallel_bootstrap for clarity
- Switch psi sources to being <j.B>/B_0
- Regenerate tests
@jcitrin jcitrin added the copybara:import-manual Set when ready for copybara manual import label Nov 27, 2025
@copybara-service copybara-service bot merged commit 8549da7 into main Nov 28, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working copybara:import-manual Set when ready for copybara manual import ready for review Add to PRs when they are ready for review unsure of who to assign as reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make conversions between toroidal and parallel current Use true <j.B> in source_profiles, rather than B_0 * j_tor

2 participants