Skip to content

Fix merge of master into implicit-fp-collisions - #376

Merged
mrhardman merged 153 commits into
implicit-fp-collisionsfrom
implicit-fp-collisions-really-merge-master
Jun 17, 2025
Merged

Fix merge of master into implicit-fp-collisions#376
mrhardman merged 153 commits into
implicit-fp-collisionsfrom
implicit-fp-collisions-really-merge-master

Conversation

@johnomotani

Copy link
Copy Markdown
Collaborator

Not sure what happened in #363. It looks like it resolved the differences with master, but didn't actually merge master. I think that's why #353 is still showing so many files changed. This PR merges master (from May 5th) into implicit-fp-collisions. When it's merged, the differences with master will look sensible.

@mrhardman making this as a PR into implicit-fp-collisions in case you were working on the #367 bug in another branch.

johnomotani and others added 30 commits March 9, 2025 15:01
Remove PyPlot from plots_post_processing to permit development withou…
Although other tests segfault with NCDatasets, HDF5 fails to precompile
when using OpenMPI_jll unless NCDatasets is also installed. NCDatasets
was originally removed from the 'debug checks' CI test only because it
didn't work for some reason, so makes sense to add it back now that it
works again on the CI server, and for some strange reason helps HDF5 to
precompile.
Install NCDatasets in 'debug checks' CI test
Actions that use julia-actions/cache should have some permissions set,
see https://github.com/julia-actions/cache#cache-retention.
Even after clearing the cache manually, the documentation workflow still
fails unless we remove the julia-actions/cache action entirely. Don't
understand why!
Disable cache in documentation workflow
This allows a quicker debug check for consistency of synchronization
calls as macros can capture the file/line where they were called.
Convert _block_synchronize() and begin_*_region() to macros, more efficient debug check for synchronization consistency
For use in the implicit electron solve,
electron_kinetic_equation_euler_update!() is parallelised over
{z,vperp,vpa} and is expected to be called inside a non-parallelised
loop over r. Therefore in the explicit advance, the loop over r must not
use the @loop_r macro.
Otherwise runs likely fail because timesteps are inconsistent on
different processes in the shared-memory block.

Also simplifies slightly the broadcasting of code_time and
pdf_electron_converged.
Use a shorter timestep to ensure the simulation does not crash when
started without a restart from a Boltzmann-electron converged,
steady-state solution.
…sation

Fix parallelisation and typo in explicit electron solve
Change is physically equivalent, although discretization changes
slightly. Updated version is more consistent with original notes
deriving moment-kinetics model, and with version that was implemented
for kinetic electrons.

Some expected values for tests that use very tight tolerances are
updated.
Increase z-resolution, decrease Lvpa (except in the split3 case),
decrease timestep.

Reduce order of vpa elements to make test more robust.

Switch the test to use `elementwise_isapprox()` instead of `isapprox()`
because `isapprox()` uses `LinearAlgebra.norm()`, which does not divide
out the number of grid points, so that the tolerances would have to be
increased when increasing the grid size.
…e at midpoint (electron steps not supported yet), which can be useful when gauging effectiveness of PI controllers - you can essentially 'see' the graphs of density and temperature motion without having to use makie_post_processing
…aming-term

Use d(g_s)/dz instead of d(f_s)/dz in free-streaming term
This will allow the time derivatives of moments to be used directly in
the kinetic equation.
Simplifies code, making fewer places that need to be updated if new
terms are added (now only RHS of kinetic equation and moment equations,
not zdot, wpadot, Fdot coefficients).

Updates expected values in some tests: discretization errors are changed
by the rearrangement of the moment kinetic advection coefficients, and
the tests are generally poorly resolved (in order to be fast), so are
sensitive to this.
We don't currently use a time advance scheme where the electron density
(equal to the ion density) is updated during the same step as the
electron pressure (because explicit and implicit steps are separate even
if the electrons are updated directly on the ion timestep), but if we
did, and also used the temperature equation option for the electrons
(instead of the default pressure equation), then the implementation
would be wrong because it did not distinguish the density at the
beginning and the end of the timestep (compare the implementation of the
parallel velocity update using a momentum equation in force_balance.jl).
This commit fixes this potential error, by passing in separate
electron_density_out and electron_density in to
electron_energy_equation!().
Not useful for substituting the time derivatives of electron moments
directly into the electron kinetic equation, because mass ratio
simplifications are done after substituting the moment equations, but
useful for diagnostics.
johnomotani and others added 26 commits April 30, 2025 16:19
atol was set too small before, causing intermittent test failures on the
CI servers.
…ils.jl

Needs updating because the `get_electron_Jacobian_matrix()` function
has all of the flags default to true, so need to set them all to false.
The changes in PR#322 increase the size of the dimensionless qpar by
sqrt(2), so the previous value of atol may now be too small. The new
version suffers from occasional faliures in
`braginskii_electrons_imex_tests.jl`, so increase the atol slightly to
avoid these.
Consistent 1D2V and 1D1V moment-kinetic equations
…ed results"

This reverts commit b1344a2 and updates
expected results of tests.
These should not have affect simulation results as the advection speeds
would be re-calculated before being used, but might possibly have
affected the initial application of velocity space boundary conditions
and also trigger the debug checks for use of uninitialized values.
* Use `elementwise_isapprox()` instead of `isapprox()`.
* Decrease the solver tolerances to very small values to make the test
  results more consistent when there are rounding errors, different
  iteration counts on different machines or numbers of processes, etc.
* Use the higher-order IMEX time-solver for the default case, as this is
  faster when using very tight time-solver tolerances.
Remove hacks needed to exactly match pre-PR#322 code
Speeds up compilation, I think by allowing the compiler to mostly
optimise out the large if-elseif-... chain.
New test script `test_scripts/check-makie_post_processing.jl` runs a
low-resolution case (similar to
`moment_kinetics/tests/kinetic_electron_tests.jl`, but with neutrals and
for very short output times) and then makes plots for its output, with
'all' plots enabled (post-processing input generated by
`generate_maximal_input_file()`).  Checks that `makie_post_process()`
runs without errors.
Previously when animating a variable and the minimum and maximum of a
variable were equal, tried to pass them to `ylims!()` anyway, which
caused an error. This commit adds a check that the minimum and maximum
are not equal before calling `ylims!()`, leaving the y-limits at the
default values if they are equal.
Optimize compilation of `get_variable()`; CI check for `makie_post_process()`
Add missing factor of 1/2 in 'Reduction to 1D1V' docs
@mrhardman

mrhardman commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator

implicit-fp-collisions is unfortunately quite out of date. The branch for #353 is implicit-fp-collisions-precon-debug. I was working on implicit-fp-collisions-merge-master when I noticed #367, but I haven't fixed that bug elsewhere since then.

@mrhardman
mrhardman merged commit 63767e0 into implicit-fp-collisions Jun 17, 2025
@mrhardman

Copy link
Copy Markdown
Collaborator

I just merged this hoping that I could then simply merge implicit-fp-collisions-merge-master into implicit-fp-collions, but I still many many merge conflicts so I conclude this was a mistake.

@johnomotani
johnomotani deleted the implicit-fp-collisions-really-merge-master branch June 17, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants