Skip to content

TGLF interface#2104

Merged
jcitrin merged 1 commit intomainfrom
tglf_interface
Apr 24, 2026
Merged

TGLF interface#2104
jcitrin merged 1 commit intomainfrom
tglf_interface

Conversation

@aaronkho
Copy link
Copy Markdown
Collaborator

@aaronkho aaronkho commented Apr 6, 2026

  • Added TGLF transport model interface, structure based on QuaLiKiz transport model
  • Added extra physics quantities to TGLF-based interface
  • Added collisionality multiplier to TGLF-based interface

@aaronkho aaronkho requested a review from theo-brown April 6, 2026 22:01
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 6, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@aaronkho
Copy link
Copy Markdown
Collaborator Author

aaronkho commented Apr 6, 2026

@theo-brown Please reassign if you think someone else should also review this.

Copy link
Copy Markdown
Collaborator

@theo-brown theo-brown left a comment

Choose a reason for hiding this comment

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

Great stuff! Excited to see this come in. There might be a couple of further rounds of review if others from the GDM team want to chip in, but we should be able to get the ball rolling.

There are few linting/style errors that should be caught by running the linter/formatter. Have you got that set up?

Also please could you investigate the failing tests and make fixes where possible?

Thanks!

Comment thread torax/_src/transport_model/pydantic_model.py Outdated
Comment thread torax/_src/transport_model/tglf_based_transport_model.py Outdated
Comment thread torax/_src/transport_model/tglf_based_transport_model.py
Comment thread torax/_src/transport_model/tglf_based_transport_model.py
Comment thread torax/_src/transport_model/tglf_based_transport_model.py
Comment thread torax/_src/transport_model/tglf_transport_model.py Outdated
Comment thread torax/_src/transport_model/tglf_transport_model.py Outdated
Comment thread torax/_src/transport_model/tglf_transport_model.py
Comment thread torax/_src/transport_model/tglf_transport_model.py Outdated
Comment thread torax/_src/transport_model/tglf_transport_model.py
@aaronkho
Copy link
Copy Markdown
Collaborator Author

aaronkho commented Apr 8, 2026

Ok I fixed the majority of the CI tests that I could. It seems the current failing test is just due to the safe_divide changing some parameters by a marginal amount. The relative tolerance for matching is 1.0e-10, but many values are in the 1.0e-9 range which is probably small enough to not cause concern.

This will either take someone else to re-run the ITER hybrid TGLFNN test case and replace the appropriate test netcdf file or I could try to remove the offending safe_divide (not sure which one is the major contributor though).

@aaronkho
Copy link
Copy Markdown
Collaborator Author

aaronkho commented Apr 8, 2026

@jcitrin As mentioned above, I added safe_divide to some quantities in the TGLF-based calculations. This causes the ITER-hybrid TGLFNN test case to fail. As far as I can tell, all other tests pass.
The runner logs show that the differences are extremely marginal but still enough to fail the test. Would someone be able to update that test case based off of this PR? Also, I've addressed the majority of the review comments from @theo-brown, what are the next steps?

@theo-brown
Copy link
Copy Markdown
Collaborator

I'll start the import process, and I can rerun the test for you.

@theo-brown theo-brown added the copybara:import-manual Set when ready for copybara manual import label Apr 9, 2026
@theo-brown
Copy link
Copy Markdown
Collaborator

What happens next is:

  1. We import it into the internal version of the repo (done)
  2. Internal review, potentially making some changes (in progress)
  3. Merge with attribution preserved

@theo-brown
Copy link
Copy Markdown
Collaborator

Have you got any sim results for e.g. the iterhybrid case?

@theo-brown
Copy link
Copy Markdown
Collaborator

This is really awesome, btw. Works super seamlessly.

Below is TGLF (dashed) vs TGLFNN-ukaea (solid).

newplot (2)

@aaronkho
Copy link
Copy Markdown
Collaborator Author

Great to see! And glad to hear that you're happy with it thus far. :)

@theo-brown
Copy link
Copy Markdown
Collaborator

Sorry for the delay in bringing it in, getting TGLF to compile within Google is a bit complicated.

@theo-brown
Copy link
Copy Markdown
Collaborator

Ok, TGLF now working internally. A few minor changes have come up in internal review, so the final merged version might have some differences. This PR will be merged soon!

@aaronkho
Copy link
Copy Markdown
Collaborator Author

Awesome! We also noticed a minor issue in the default values set in tglf_transport_model.py, specifically the defaults should be use_bpar = False; use_bper = True, but we accidentally coded it in the other way around (use_bpar = True; use_bper = False). Can you correct this on your internal version?

@copybara-service copybara-service Bot mentioned this pull request Apr 24, 2026
copybara-service Bot pushed a commit that referenced this pull request Apr 24, 2026
FUTURE_COPYBARA_INTEGRATE_REVIEW=#2104 from google-deepmind:tglf_interface 62801d2
PiperOrigin-RevId: 905087246
copybara-service Bot pushed a commit that referenced this pull request Apr 24, 2026
FUTURE_COPYBARA_INTEGRATE_REVIEW=#2104 from google-deepmind:tglf_interface 62801d2
PiperOrigin-RevId: 905087246
@jcitrin jcitrin merged commit 4d3fed2 into main Apr 24, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copybara:import-manual Set when ready for copybara manual import

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants