Merge separate-postprocessing-packages into #166 #178
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avoids needing to import unnecessary dependencies (Plots.jl when using Makie.jl, or Makie.jl when using Plots.jl, or either when only running simulations), which should reduce precompilation time, and the time to build system-images. Also removes a few unused dependencies.
It is inconvenient to provide the option for runtime plots when splitting the post-processing functionality into separate packages. The option was not used much anyway. It should be possible to add an 'extension' (with Julia-1.9.x) that provides the option if a post-processing module is loaded, if we want to add it back in future.
Allows us to separate the Project.toml for the moment_kinetics package from the Project.toml that a user sets up to run the code.
NetCDF file I/O is now done by an 'extension' that is available only if NCDatasets is installed.
The 'generic' definitions are intended only to define a name that can be imported by each implementation to add an implementation-specific method. If they are defined with brackets after the name (as several were previously) then there is a zero-argument method of the function that does nothing, which might sometimes be confusing. Therefore remove those brackets.
The TimerOutput argument is not needed any more, so do not need the TimerOutputs dependency.
This lets us use them in the tests.
This function is not available as Plots.jl is not included as a dependency of moment_kinetics any more.
Also bumps the Julia version used for the CI to 1.9, as this is required
in order to use package extensions.
Also adds NCDatasets to test dependencies. This will require/install
NCDatasets when running `Pkg.test()`, but it should still be possible to
run the tests without NCDatasets by just doing
`include("moment_kinetics/test/runtests.jl")`.
The 'modern' way to specify extra dependencies for the tests in Julia is using `test` in the `[targets]` section of the top-level Project.toml. The old method of using an extra `test/Project.toml` is described as 'the old method' which, while it will continue to be supported in Julia-1.x, seems to be out of favour. The main point of this update though is to remove extra dependencies from the tests, which are not needed.
NCDatasets is now an optional dependency (also Symbolics, but that is not used in the test suite). To allow the tests to run when NCDatasets is not installed, the tests that use NetCDF now fall back to HDF5 if NetCDF is not available. The fall-back can be disabled explicitly by passing the --force-optional-dependencies command line flag, to ensure that the CI tests do actually use NetCDF I/O, and will error if it is not available.
macOS parallel tests are very slow (at the moment), so only run on 4 processes (not 3 or 2), and do not run long tests.
Updates the machine setup scripts in the `machines/` subdirectory and the precompilation scripts to account for the new package structure (with postprocessing tools moved into separate packages). Also adds a 'generic-pc' option that can be used to set up moment_kinetics on a standard Linux desktop/laptop machine.
This is convenient when the user chooses to have their .julia locally in the project directory.
When not on a cluster with a batch job submission system, the shell scripts to submit jobs are just clutter in the top level directory. Move the scripts to machines/shared, and symlink them at the top level when we set up moment_kinetics for a batch system.
This change puts all the prompts where interactive user input is required to the same place, near the beginning of the script, so that the user can go away and do other things while (potentially) slow parts like precompilation are running.
Allows for setting up separate project directories for post-processing, which will be useful for example when the different projects need different optimization flags.
Ensures precompilation is done with the same flags as will be used later (or at least should be). This is important as precompilation of dependencies will be (or may be) re-done if the flags change.
This should be simpler than maintaining a separate script for each machine. Now machine-specific settings are saved in a machine_settings.toml file for each configuration.
Also simplifies the finding of the julia executable. The positional argument to machine_setup.sh and the '-d' flag are removed - the script just prompts the user for input, and remembers the settings so that they are provided as the default if the machine_setup.sh script is re-run.
The MPI and HDF5 setup is no longer done in the machine_setup_stage_two.jl script. It is also not really expected that the script will be run other than from machine_setup.sh. So the message saying Julia is exiting and the forced exit by calling `exit(0)` are not really necessary.
Thanks to fixes that mean julia-1.10 should work better with MPI, it is now the recommended version, so use julia-1.10 for the CI automated tests.
Rounding error sometimes causes chebyshev_pseudospectral matrix-multiplication derivative test to fail on macOS, so use 2e15 instead of 1e15 for the tolerance.
This will allow the function to be used in moment_kinetics, makie_post_processing, or plots_post_processing.
If a source code file is found that is newer than the system image file, can stop checking.
...and when running post processing.
When $JULIA_DIRECTORY is empty, need to no set $JULIA_DEPOT_PATH.
Previously the imports of Glob and Primes packages required that `moment_kinetics` was the active project, not just added as a dependency.
There may be some routines that follow different code paths for the first or last element, so try to make sure all code is used.
Means collision operator code gets compiled and added to system image `moment_kinetics.so`.
…-merge-geometry-upgrade
Needs to be a bash script so we can activate the Python venv within it when enabling plots_post_processing.
|
I started attempting this myself here https://github.com/mabarnes/moment_kinetics/tree/geometry-upgrade-separate-packages. The automatic CI tests on that branch did compile and run on Julia 1.9.3 The first thing I can see in conflict with the way I had to introduce geometry was that I had to split up the "get_geometry_and_composition" function because I didn't want to make the MMS test scripts unnecessarily reproduce the geometry arrays. There is a lot of nice things to be said for using the same functions in post processing as in the main code, but it is also a pain if one has to inherit a lot of data that isn't available to the script. |
Ah, sorry, I'll add that back - I missed that Anyway, there weren't too many conflicts, so hopefully either version of the merge should be fairly easy to polish up. |
johnomotani
force-pushed
the
separate-postprocessing-packages-merge-geometry-upgrade
branch
from
6bf2597 to
c0bf27f
Compare
johnomotani
deleted the
separate-postprocessing-packages-merge-geometry-upgrade
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.