Fs subcycle cfl fix #307
Merged
Fs subcycle cfl fix #307
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
… solver. Extremely slow as long time spent in allreduce itself.
Slowness is in large part due to load imbalance.
Also removed the writeVerbose commands.
No changes, passes testpackage against bc34c4d. Magnetosphere has the following, small random diffs which are unavoidable. Magnetosphere_small - Verifying bc34c4deaadfb99491b72b26c68545c1196dd7ab___DACC_SEMILAG_PQM__DTRANS_SEMILAG_PPM__DDP__DDPF__DVEC4D_AGNER against df2ca359d2ed439613496c4819a649572162c1cc__DACC_SEMILAG_PQM__DTRANS_SEMILAG_PPM__DDP__DDPF__DVEC4D_AGNER -------------------------------------------------------------------------------------------- ------------------------------------------------------------ ref-time | new-time | speedup | ------------------------------------------------------------ 48.26 49.58 0.973376 ------------------------------------------------------------ variable | absolute diff | relative diff | ------------------------------------------------------------ rho_0 9.55033 9.33096e-06 rho_v_0 4.88038e+06 9.76061e-06 rho_v_1 165343 8.39368e-06 rho_v_2 160637 9.98949e-05 B_0 1.97057e-15 4.93907e-07 B_1 2.73734e-15 6.79429e-07 B_2 3.56442e-15 2.13865e-10 E_0 5.79479e-09 2.59921e-06 E_1 5.91271e-09 9.21795e-07 E_2 4.06094e-09 2.27526e-06 -------------------------------------------------------------------------------------------- Distribution function diff -------------------------------------------------------------------------------------------- INFO Reading in two files. File names: /stornext/field/vlasiator/test_package_data/df2ca359d2ed439613496c4819a649572162c1cc__DACC_SEMILAG_PQM__DTRANS_SEMILAG_PPM__DDP__DDPF__DVEC4D_AGNER/Magnetosphere_small/bulk.0000001.vlsv & /lustre/tmp/yann/testpackage/run/Magnetosphere_small/bulk.0000001.vlsv NonIdenticalBlocks: 0 IdenticalBlocks: 3321 Absolute_Error: 1.89792e-18 Mean-Absolute-Error: 7.63565e-23 Max-Absolute-Error: 1.92028e-20 Absolute-log-Error: 6.45948e-05 Mean-Absolute-log-Error: 2.59876e-09
Conflicts: fieldsolver/ldz_electric_field.cpp
One does not work yet, this is work in progress.
Added map include so that it compiles on sisu
|
Can you for clarity summarise at the top what this is about? It is easier than clicking through to an old/other pull request and reading through an extended thing there... |
A new, dynamic timestep is actually being calculated, but then discarded.
A new, dynamic timestep is actually being calculated, but then discarded.
And give a friendly message about it being overridden.
…vements Field solver stepping improvements. Everybody agrees that this looks good now, and it's about time we got this into master.
And give a friendly message about it being overridden.
Updated testpackage (subcycle tests and polar magnetosphere)
Fix update moments in acc I conclude: Substeps look just as bad as the same amount of normal steps, this pull request doesn't alter them significantly, all is good.
|
This pull request is a makeover of the fieldsover subcycle dt calculation, to ensure that it never oversteps and doesn't do too few steps, either. Original found during the "summer of debugging", where field artefacts were occuring on subcycle-amount-boundaries. Fixed by yann, then fixed again in a different way by sebastian (hence the large amount of commits), then rebased onto master by urs. |
Conflicts: fieldsolver/ldz_main.cpp
|
The derivatives of the moments needn't be recomputed in the second RK step of any subcycle, not even during the first subcycle. |
|
The updated testpackage shows 0 changes, except in the polar magnetosphere test and (ridiculously tiny) changes in the fieldsolver substep test: so I assume this is really just caused by floating point-accuracy level changes. |
ykempf
reviewed
Feb 23, 2017
fieldsolver/ldz_main.cpp
Outdated
| // If we are at the first subcycle we need to update the derivatives of the moments, | ||
| // otherwise only B changed and those derivatives need to be updated. | ||
| calculateDerivativesSimple(mpiGrid, sysBoundaries, localCells, RK_ORDER2_STEP2, (subcycleCount==0)); | ||
| // Here, we never need to update the moment derivatives since they will not have changed from |
There was a problem hiding this comment.
That is, until we interpolate moments in time as Sebastian wants us to do since last summer.
There was a problem hiding this comment.
True, but that doesn't happen yet, so this is, for now, correct.
I would suggest merging this now, if there are no other objections.
ursg
added a commit
to ursg/vlasiator
that referenced
this pull request
Feb 23, 2017
…uired since within the substeps, the moments are being accessed at both RK2 times alternatingly, they need to be computed back and forth. This *might* cost some performance, or then again it might be negligible. If it is to costly, they could also be precomputed and stored.
ursg
added a commit
to ursg/vlasiator
that referenced
this pull request
Feb 24, 2017
…uired since within the substeps, the moments are being accessed at both RK2 times alternatingly, they need to be computed back and forth. This *might* cost some performance, or then again it might be negligible. If it is to costly, they could also be precomputed and stored.
ykempf
added a commit
that referenced
this pull request
Feb 24, 2017
Revert last commit of #307: Moment derivatives are actually required.
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.
This replaces pull request #229, it was simply rebased onto current master.
The testpackage runs fine with it, but that is unsurprising, since the testpackage doesn't subcycle.