-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add Vloop BC #456
base: main
Are you sure you want to change the base?
Add Vloop BC #456
Conversation
I have not yet written a test case for this, as I wanted to run the pattern past one of the team first. Let me know your initial thoughts and then I can continue! |
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.
Some general thoughts.
- What's missing is a consideration of how to initialize psi to begin with, for all cases. If Ip is None, then how do we set the right boundary condition at the first timestep, for cases where we initialize based on plasma current? Recall that there are several options currently (which can be further extended later):
i) profile_conditions['initial_psi_from_j'] = False, i.e. Initialize psi based on the input geometry file. Two suboptions:
a) geometry['Ip_from_parameters'] = False.
Here's it's clear what to do, psi_right = psi_geometry[-1]
b) geometry['Ip_from_parameters'] = True. For this we need Ip to rescale psi, so if Ip=None we have a problem
ii) profile_conditions['initial_psi_from_j'] = True, i.e. Initialize psi based on the "nu" current profile model. This needs normalization to a total current, i.e. Ip. So if Ip=None we have a problem. Note that at the initial condition, the Vloop_bound_right needs to be such that Psi.face_grad()[-1] is consistent with Ip, according to the same formula as the standard boundary condition. See fvm.cell_variable.face_grad, that face_grad()[-1] is just a standard derivative between the dx/2 distance between the last cell point and the rightmost face.
To solve this, I recommend changing the name of Ip to Ip_input, and for cases where the Vloop boundary condition is used, then it's used (if necessary) to initialize psi. That means that Vloop_bound_right and Ip can coexist. The pattern should be a bit different, e.g. an extra boolean "use_Vloop_boundary_condition" or something like that. Note that Ip_input can still be a TimeInterpolatedInput, but for the Vloop case only the initial condition at t=t_initial will be used.
- We need to adjust to the Vloop boundary case where profile_conditions['Ip_input'] is not actually total current, but rather the initial condition. For example, the output Ip is taken from there, and also in plotruns_lib.PlotData.i_total. For the code to work with both boundary condition cases, we should take Ip (total current) from elsewhere. A candidate could be to extend physics.calc_jtot_from_psi to output I_tot as well (can change the name to Ip_profile), and then save this quantity in CoreProfiles.Currents . Total current is then Ip_profile[-1]. This can be a separate PR. This way, we also get information on the total current evolution in the Vloop boundary condition case.
Great points, and this is exactly why I wanted to run it through you! I think because for the type of simulation I've learnt how to do we always prescribe psi from a geometry I hadn't really thought about the consequences for any situation other than 1ia). Noting down for myself - I need to consider the cases:
and their interactions with the Vloop BCs |
#492 was done with this PR in mind |
93fd0aa
to
3b7f33c
Compare
I've been working on incoporating the Vloop BC via
In this case, I've attempted to sidestep the initialisation problem by having a spatial-derivative BC for the first timestep and a value BC for subsequent timesteps. I've found that my current method results in an inconsistency with the setting of psi vs psidot. From the code, it appears that within Lines 1568 to 1583 in c860557
torax/torax/sources/ohmic_heat_source.py Lines 43 to 138 in c860557
I don't understand yet what's going on in |
Just to check on this, do you mean a property of |
72e1f0e
to
0051d37
Compare
Makes most sense for it to be in CoreProfiles |
0051d37
to
bcdbb04
Compare
This PR is now ready for review :) Summary of changes
Outstanding questions/bugs
|
Not sure why this failed copybara - is it because it's behind main so there are conflicts? |
Indeed, it's due to merge conflicts. Strongly recommend merging main and syncing to HEAD. It may get a bit hairy, sorry about that. 😬 And apologies again for the delays in review. Will get on it tonight |
OK! I'll give it a go this morning. |
ca8a3a2
to
50f087b
Compare
Vloop (= dpsi_edge/dt) can be a useful BC for scenarios where the voltage swing of the central solenoid is directly controlled (e.g. in a fully non-inductive scenario, Vloop = 0). Summary of changes: - Added new BC for the psi equation. - Can be combined with any of the psi initialisation methods: - Prescribed psi + Ip_tot rescaling - Psi from geometry file - Psi from geometry file + Ip_tot rescaling - Psi from nu formula + Ip_tot rescaling - Added vloop_lcfs to state.CoreProfiles, which is a direct copy from psidot.face_value()[-1]. - Required adding the core profiles from the previous timestep to the boundary condition computation - Added psi_right_bc and vloop_lcfs to output.
50f087b
to
bd03157
Compare
I've rebased onto main, not sure why it's still failing. |
Yeah, weird. Maybe it's due to merge conflicts in the .nc files which it doesn't know how to resolve. We can resolve them manually when we do the manual import and internal review, so I wouldn't worry about it. As long as it's rebased to main and the actual tests are passing, should be fine. |
Adding Vloop boundary condition on the psi equation, so that the current can be evolved self-consistently given a rate of change of edge flux.
Tasks: