-
Notifications
You must be signed in to change notification settings - Fork 4
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
Krook collisions manufactured solutions test port #141
Conversation
…est -- currently the tests do not pass.
Options for manufactured solutions were moved from 'composition' to a dedicated NamedTuple. However, the fields in the 'composition' struct were not removed - remove them, and fix bugs caused by those fields still having been used, although they were not being set correctly from the input options.
These plots are uninteresting, and cause errors because the y-axis limits are both 0.
…ntion in master, which seems to normalise pressure to 2 nref Tref rather than nref Tref = (1/2)mref nref cref^2.
…function to use get_density, get_upar and get_ppar functions. In the split moment operation, the moments passed in to these functions along with the distribution are set to the enforced values of (density,upar) = (1.0,0.0) appropriately. This branch now supports the test_velocity_integrals.jl script.
…dy of the code
…ere pperp (and thus vthi) are computed incorrectly according to the manufactured solutions test in 1V and 2V.
Needs the factor of 2 to be consistent with normalisations currently used.
Need larger boxes to make moments accurate to roughly machine precision.
…the output binary.
Using just '.' means the contents of '.julia' get put directly in the top level of the repo - really want to create a '.julia' subdirectory within the repo.
A recent update to NCDatasets.jl changed the behaviour of indexing like `var[:]` when loading a NetCDF variable. Now `var[:]` returns a 1d view. Fix by using `copy()` to convert the `NCDataset` to an `Array`.
… implementation. Suited only for 1D1V with standard drift kinetics for now. Errors suggests an order unity factor difference somewhere in the two implementations.
Better suggestion for putting .julia locally within the repo in `machine_setup.sh`
…he present branch for the 1D1V case. It is noted that the manufactured solutions test passes when the Krook collision frequency is a constant independent of position. With this in mind, I have modified how the Krook operator is specified in the input file. Now one should specify "krook_collisions_option" ("default" -- use the frequency from the reference parameters and use the spatially dependent factor n/vth^3 -- or "manual" -- use a frequency set in the input file). The frequency can be set in the input file with the "nuii_krook" parameter, if the krook_collisions_option = "manual". The tests and previous functionality are preserved.
…hermal_speed written to the HDF5/netCDF file is correct.
…ant frequency coefficient).
…k operator. Check-in tests still fail for undiagnosed reasons.
d327726
to
a8e314d
Compare
Using post_processing.jl I get an out of bounds error because of the interaction of the following two lines moment_kinetics/src/coordinates.jl Line 218 in a8e314d
and moment_kinetics/src/moment_kinetics_input.jl Line 371 in a8e314d
@johnomotani What was the resolution in makie_post_processing.jl? |
Ah, I didn't realise that error affected the post_processing.jl too. I have a fix somewhere, will cherry-pick onto this branch in a min. |
This is needed since 'sqrt spacing' option was added. Also include velocity_integral_tests.jl in runtests.jl so that it is included in the CI tests. Also remove some println() statements that had been left in by mistake.
Set irank/nrank as if running in serial, to avoid errors from set_element_scale_and_shift()).
The most recent set of test failures seem to have been because somehow the Krook operator was being turned on by default. After making sure it is off by default, the tests pass on my laptop. Fingers crossed the CI is OK again now. |
Thanks for correcting my mistake! |
I confirm that I am now also seeing the MMS test behave well for It looks like the tests are still failing on CI, but they pass for me locally. |
I don't think the test failures here are real. The CI servers just seem to be going super slowly today, and timing out while they are still precompiling the dependencies. |
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.
Looks good to me. Just a couple of tiny choices left.
This option now passes the tests, so might as well make it the suggested option.
Pull request that provides a manufactured solutions test for the charged-particle Krook collision operator in 1V and 2V. These changes also required pperp to be added to the moments and diagnostics, and upar, ppar, pperp, and vth are now tested in the post_processing.jl manufactured solutions test script.
The manufactured solutions test shows large errors when the collision frequency depends on the local density and thermal speed. This may be due to an instability. To make progress in finite time, I have added a Krook collision operator option that has a constant (in space) collision frequency. With this option, the manufactured solutions test behaves well.
I have not supported evolving split moments options.