Replaced user iterations of EqulibriumTransport actor with convergenc… #210
Conversation
…e criteria Two changes were made: 1. Moved SteadyStateCurrent actor to before prepare step in EquilibriumTransport workflow 2. Replaced option of user-input number of iterations with a convergence criteria for EquilibriumTransport runs with CHEASE
Co-authored-by: Tim Slendebroek <[email protected]>
Co-authored-by: Tim Slendebroek <[email protected]>
|
I do not understand why the CHEASE equilibrium solver should be treated any different from any other equilibrium solver. |
The issue that we were trying to solve here is that when rescale_eq_to_ip is set to true, the rescaling isn't very accurate after just one iteration. By iterating multiple times with the rescale option forced to true, the j_tor profile gets closer and closer to the value of Ip. The reason it's specific to CHEASE is just because Solovev doesn't have the rescale_eq_to_ip option available, so it doesn't have the same convergence problem. |
|
If it does not have the same convergence problem, then your convergence criteria should be automatically satisfied and Solovev should only run once. |
Removed duplicate code by allowing runs with Solovev to go through the same while loop as runs with CHEASE. Because Solovev loses current over successive iterations, forced runs with Solovev to only iterate once
…es/FUSE.jl into eqtransport_convergence
src/actors/compound_actors.jl
Outdated
| @@ -57,18 +57,52 @@ function _step(actor::ActorEquilibriumTransport) | |||
| # Set j_ohmic to steady state | |||
| finalize(step(actor.actor_jt)) | |||
|
|
|||
| for iteration in 1:par.iterations | |||
| @assert act.ActorCHEASE.rescale_eq_to_ip "Running CHEASE with ActorEquilibriumTransport requires ActorCHEASE.rescale_eq_to_ip = true" | |||
There was a problem hiding this comment.
I am wondering if this @Assert is necessary, since we already do a act_chease = deepcopy(act.ActorCHEASE) we could then simply force act_chease.rescale_eq_to_ip = true. Is there any problem with that?
There was a problem hiding this comment.
I switched out the assert for an if statement that hard resets rescale_eq_to_ip to true if it gets false, but also gives the user a warning that it did so. I thought this made sense so that if someone did try setting it to false, they would know that it's actually running with true. Does that work?
src/actors/compound_actors.jl
Outdated
| avg_diff = sum(abs.(j_tor_after .- j_tor_before)) / length(j_tor_after) | ||
|
|
||
| if act.ActorEquilibrium.model == :Solovev | ||
| avg_diff = 0 # temporary fix to force Solovev to run exactly once |
There was a problem hiding this comment.
Hopefully this gets resolved with the MXH equilibrium 🤞
src/actors/compound_actors.jl
Outdated
| max_iter = 5 | ||
| iter = 0 | ||
| ip = dd.equilibrium.time_slice[].global_quantities.ip | ||
| conv_criteria = ip / 1e2 |
There was a problem hiding this comment.
As discussed with @TimSlendebroek the convergence criterion should look at both the current and pressure profiles, taking their difference at the beginning and end of the loop.
Co-authored-by: Orso Meneghini <[email protected]>
Co-authored-by: Orso Meneghini <[email protected]>
Changed convergence criteria to an error quantification of j_tor and pressure and also removed assert for CHEASE.rescale_eq_to_ip = false in favor of a warning
…es/FUSE.jl into eqtransport_convergence
Co-authored-by: Tim Slendebroek <[email protected]>
…e criteria
Two changes were made: