Skip to content
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

Implement BoutDataset.to_restarts() method #117

Merged
merged 19 commits into from
Sep 9, 2020
Merged

Conversation

johnomotani
Copy link
Collaborator

@johnomotani johnomotani commented Apr 20, 2020

Allows restart files to be created from a BoutDataset, for any time-index in the dataset and for any valid nxpe, nype.

Based on parallel-interpolation in order to avoid merge conflicts (although the conflicts are trivial so I could rebase easily if that's useful), and because the motivation for adding this now is to be able to use interpolate_parallel() to create a higher-resolution Dataset, and then create restart files from the interpolated data.

Could do with adding a test for this, at least writing out the restart files and then reading them back in and checking the fields are as expected.

  • test added

@johnomotani johnomotani added enhancement New feature or request work in progress Not ready to merge labels Apr 20, 2020
@pep8speaks
Copy link

pep8speaks commented Apr 20, 2020

Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-09-09 13:51:02 UTC

Allows selecting only the needed variables, reducing the size of the
resulting restart files.
Creates restart files from a certain time-slice of a Dataset.
It's unlikely that a user will want method chaining with an output
method, and returning the Dataset means we get a lot of printed output
if we just call 'ds.bout.to_restarts()'.
@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2020

Codecov Report

Merging #117 into master will increase coverage by 0.24%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   77.64%   77.88%   +0.24%     
==========================================
  Files          14       14              
  Lines        2035     2139     +104     
  Branches      452      480      +28     
==========================================
+ Hits         1580     1666      +86     
- Misses        299      304       +5     
- Partials      156      169      +13     
Impacted Files Coverage Δ
xbout/boutdataset.py 75.17% <33.33%> (+0.76%) ⬆️
xbout/utils.py 80.49% <78.64%> (-1.07%) ⬇️
xbout/region.py 87.32% <0.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05f79ba...606bd65. Read the comment docs.

- Set data_vars="minimal" in concat() when combining boundary_pad with
  ds - ensures that variables without an x- or y-dimension do not have a
  new dimension added.
- Make boundary_pad a copy of the boundary cells, then we can set to NaN
  instead of using the strange expression with 'where()'.
- Call .load() on both boundary_pad and ds before combining. Ensures
  that concat() with data_vars="minimal" does not fail because of
  different dask-states of the underlying data arrays.
xarray requires less-than-6-months old dask, so 0.16.0 requires
dask-2.10.
Probably more 'correct' and needed to make to_restart() tests work.
Most of checks need to check numbers divisible by MXSUB or MYSUB, not
NXPE or NYPE.
BOUT++ does not require ixseps1 or ixseps2 to be on processor
boundaries, so allow any splitting in x where nxpe divides nx-2*mxg
This is the most common case, so better to test this. Also tests more of
the code as some functions are only used when there are guard cells.
Leaving out no-guard-cell case just to save time on the tests (it's not
commonly used).
@johnomotani johnomotani merged commit ec95524 into master Sep 9, 2020
@johnomotani johnomotani deleted the create-restarts branch September 9, 2020 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request work in progress Not ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants