-
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 Vloop BC #456
base: main
Are you sure you want to change the base?
[WIP] 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 |
This includes a hack to support outputting different BCs in the output file. Previously, the simulation could only use one type of BC for the whole run (ie either grad or value constraint). By wrapping the output in a jnp.array(), BCs that are None get turned into NaN, which is compatible with tree_map. Hence, this change allows you to have `grad_constraint = [XXX, None, None, ...]` and `value_constraint = [None, XXX, YYY, ...]` which is useful for testing the Vloop BC.
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
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.
I have:
config["profile_conditions"]["Vloop_bound_right"]
for the potentially time-varying Vloop at LCFS.Madeconfig["profile_conditions"]["Ip"]
andconfig["profile_conditions"]["Vloop_bound_right"]
mutually incompatible (with unit test)right_face_constraint
to the Psi element ofcompute_boundary_conditions
, set byVloop_bound_right
This necessitated the following changes:
dt
intocompute_boundary_conditions
Remaining tasks: