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

Add extra variables #2

Conversation

AaronDavidSchneider
Copy link

Hi @cspencerjones,

I tried to address the changes requested in MITgcm#205 by adding a test and extending the docstring.

Cheers
Aaron

timothyas and others added 29 commits April 14, 2020 11:03
* access to llc2160 grid is a go, with a couple caveats for now

* use grid var names not filenames, get vertical grid vars working

* lil typo

* fixes loading grid dir issues

* it works for llc2160

* add directory to some LLC4320 grid vars

* separate grid metadata function, add shrunk_grid flag

* add grid_varnames to paramter list

* test that grid variables show up in portal datasets

* prefixes->variables

* try Zl...

* extra logic to create Zl, Zu, and Zp1

* make reading grid optional

* wow, not copying dicts really messes things up

* only build Zu, Zl if RF is there

* shouldn't kp1 have nz+1 levels

* only read grid if grid_varnames has entries

* also test selected k_p1 levels

* typo...

* ... more typos...

* rhoref, extra dx/dy metrics

* add new metrics to test

* typos, and add rhoRef to vgrid prefixes

* load vgrid as one big chunk, all at once. test it too!

* oops, fixed test

* fix for global vgrid vars... what about layers.. tbd

* cleanup vgrid check

* cleanup grid var dict situation

* also test latlon vgrid coord chunking

* faces->ll

* update docs

* tiny typo

* added more 4320 grid files
* add xg and yg to llcreader known_models

* add other grid variables
* init gha files

* add aiohttp

* use cache v2 and goanpeca->conda-incubator

* trying to recreate GHA error...

* try sorted lists?

* try without aiohttp on py27

* no need for travis yml

* no need for coveragerc
* attempt to fix py27 with old zarr

* alternate soln with setup

* didn't work, back to first soln
* empty commit to trigger CI

* add test for k_levels=[1]

* add explicit test for MITgcm#233

* test for bug in llc90

* fixes MITgcm#224

* resolve final bugs
* very rough draft .... almost have all tiles correct...

* bering strait working

* all faces accounted for ... now to code it better...

* quick prefix fix

* look for meta file for data prec, otherwise use class default

* debugged ... was looking in grid dir

* to aws

* prefix bug

* give standard dimensions some attrs for xgcm

* temporary aws changes

* look for extra grid var filename prefixes

* semi general padding for non shrunk fields

* slightly better

* lots of variables

* added KPP nonlocal salt/temp flux

* dtype from meta prop to dask array creation

* some small bugs...

* provide dtype apriori, reading on the fly too slow. revert utils

* semi better handling of index key

* no latlon for aste

* specify iters for nonuniform step

* aws nested store, specify iters, add tests

* tests actually pass locally...

* python 3 prints

* ASTE documentation

* pretty it up

* hopefully clearer about iters vs iter params

* change some wording

* update ref names and order

* fixed up padding logic

* update that aste ref ... again

* get my face back!

* _pad_facet sufficiently documented...with pics
* feat: IMPLEMENTS CHAECKING FOR VARIABLE MATES

On a latlon grid some variables have mates. This commit will produce
a meaningful error if attempting to use `get_dataset` or
`faces_dataset_to_latlon` to load a variable without its mate.

See Github issue MITgcm#234

* test: ADDED TEST FOR THE MATE ERROR

* test: REMOVED TEST FOR VECTOR MATE ERROR

removed test for vector mate error to determine whether CI problems are
coming from the test or an xarray-master commit

* fix: ADDED TEST BACK AND CHANGED ARGUMENTS
…ITgcm#236)

convention here seems to be

    XC=dict(dims=["j", "i"], attrs=dict(
        standard_name="longitude", long_name="longitude",
        units="degrees_east", coordinate="YC XC")),

some instances fixed where "coordinate" was spelled "coordinates".  May have no effect.
…gcm#240)

* feat: CHECKS ITERATION PARAMETERS WHEN GETTING LLCMODEL DATASETS

When loading datasets using the llcreader the user is able to specify
the iteration start, stop and step. These can also be specified as
attributes in the `BaseLLCModel`. This commit introduces a check that
compares the `iter_start` and `iter_step` fed into `get_dataset` with
the `BaseLLCModel` attributes. If they are found to be incompatible
a warning is triggered.

A warning over an Error was chosen as subsuquent code may still execute
successfully even if there is a mismatch between the user specified and
`BaseLLCModel` derived values for `iter_start` and `iter_stop`.

* refactor: CHANGED WARNING TO RUNTIMEWARNING

* test: ADDED TESTS FOR ITERATION CHECKS

* feat: ADDED CHECK ITERS FUNCTION

* feat/refac: REORDERS LOADING AND CHECKING OF ITERATION PARAMS

Currently if one tries to load an ECCO portal model by specifying iterations, an error is raised. This is due to the model having attributes `iter_start`, `iter_step` and `iter_stop`. If the user tries to call `get_dataset` with a list `iters` the function
first reads in the `iter_params` from the model's attributes. The function then goes on to check that none of the `iter_params` are defined. Upon finding they have been defined the function raises an error.

The refactor does the following:
(1) Check to see if the user has set any `iter_params` in the function call.
    (i) If they have, check the user hasn't set `iters`. If the user has set iters there's a problem, so raise an error.
    (ii) If no `iters` are set then use the user set `iter_params`

(2) If no user set `iter_params` try reading in a user set `iters` and use this

(3) If still no luck, try reading the model `iter_params`. If any of them are `None` keep going...

(4) Finally try reading the model `iters`.

(5) If nothing has been found then raise an error about the lack of iteration instructions.

(6) Check the `iter_params` or `iters` for consistency with the models attributes.

* test: UPDATED TESTS

- Refactored the ecco portal tests
- Added an aste portal test

* fix miniconda action

Co-authored-by: Ryan Abernathey <[email protected]>
* py36->py37 for latest xarray

* filename

* drop ci for 2.7, 3.6. Add 3.9

* update url, drop 2-3.6

* specify python>=3.7

* update dependencies in docs/readme
* fix variable metadata typos

* Update variables.py

* Update variables.py

some more adjoint fixes

* Update variables.py

* fix two more instances of sloppiness

* replace "psu" by "g/kg"

Co-authored-by: Timothy Smith <[email protected]>
Co-authored-by: Martin Losch <[email protected]>
* look for grid path before trying to read, allow user to provide grid_path

* add grid paths

* remove grid_path arg

* first try making sverdrup, then do portal

* same thing on Pleiades

* move sverdrup/pleiades tests to own module etc

* remove extra test module, modify model fixture
* add read_CS_chunk

* Fixing style errors.

* add read_CS_chunk

* fix tests

* ny in terms of global ny, passes test_read_mds

* Fixing style errors.

* Fixing style errors.

* passes more tests

* Fixing style errors.

* all test_utils passed

* 3 tests to go

* Fixing style errors.

* fix typo

* revert to chunks

* test read_CS_chunks, get_grid_from_input working for CS

* add figshare dl link

* fix ny size

* read_mds works

* tests utils fixed

* mds_store.py should now work with cubed sphere grid

* fix tests CS

* add possibility for nx>ny

Co-authored-by: stickler-ci <[email protected]>
Co-authored-by: Aaron Schneider <[email protected]>
* Update usage.rst

* Update usage.rst

* address atmospheric configurations
@AaronDavidSchneider
Copy link
Author

Hi @cspencerjones,
if you do not have the time to continue on this PR, I would be happy to create a new PR at xmitgcm.
Cheers

@cspencerjones
Copy link
Owner

Sorry - totally slipped my mind! All the builds are failing though. Can you have a look at that? I will try to pay more attention this week.

@cspencerjones
Copy link
Owner

And thanks for working on this @AaronDavidSchneider ...I remember it being a pretty difficult thing to do!

@AaronDavidSchneider
Copy link
Author

Hi @cspencerjones,
thanks for your message. The problem was that there are some incomplete test datasets. Should be fixed now (hopefully)

@AaronDavidSchneider
Copy link
Author

AaronDavidSchneider commented May 10, 2021

The errors are not related to this function as far as I can see

@cspencerjones
Copy link
Owner

I agree and I'm a bit lost about where these are from - I'll merge and we can try to figure it out in the main PR.

@cspencerjones cspencerjones merged commit f26ef84 into cspencerjones:add_extra_variables May 10, 2021
@AaronDavidSchneider AaronDavidSchneider deleted the add_extra_variables branch May 10, 2021 16:48
@AaronDavidSchneider
Copy link
Author

Thanks!

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.