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

New environment for oceanography image on SciServer #222

Merged
merged 19 commits into from
Feb 23, 2022

Conversation

asiddi24
Copy link
Collaborator

@asiddi24 asiddi24 commented Feb 16, 2022

Updating the environment file for the new oceanography image.

Will keep committing changes to this while we work on updating the package list.

Closes #220
Closes #221

sciserver_catalogs/environment.yml Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@asiddi24 asiddi24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installing oceanspy with git and keeping xesmf<0.5.0 seems to have done the trick !

@Mikejmnez
Copy link
Collaborator

Mikejmnez commented Feb 17, 2022

@asiddi24 I haven't tested this environment with the notebooks on Sciserver, did you?

Also, tests are failing, which setting cartopy<0.20 should have fixed. The PR #218 passes all tests. I would combine the additional changes there to ensure the tests pass.

@asiddi24
Copy link
Collaborator Author

Yes ! I'm almost finished testing it. The tests are passing and all the notebooks are running, except the Kogur one which is taking a bit of time. Once it finishes, I think we're all set then to send the environment to Mitya.

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #222 (a22ea43) into master (0e19a23) will increase coverage by 1.39%.
The diff coverage is 89.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #222      +/-   ##
==========================================
+ Coverage   95.30%   96.69%   +1.39%     
==========================================
  Files          10       10              
  Lines        3809     3569     -240     
  Branches      849      766      -83     
==========================================
- Hits         3630     3451     -179     
+ Misses         99       67      -32     
+ Partials       80       51      -29     
Flag Coverage Δ
unittests 96.69% <89.75%> (+1.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
oceanspy/open_oceandataset.py 97.53% <ø> (ø)
setup.py 0.00% <ø> (ø)
oceanspy/_ospy_utils.py 98.19% <50.00%> (+0.02%) ⬆️
oceanspy/utils.py 96.72% <66.66%> (ø)
oceanspy/_oceandataset.py 97.58% <87.50%> (+0.01%) ⬆️
oceanspy/llc_rearrange.py 87.59% <90.22%> (+4.66%) ⬆️
oceanspy/subsample.py 97.97% <100.00%> (ø)
... and 1 more

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 68b0f00...a22ea43. Read the comment docs.

@asiddi24
Copy link
Collaborator Author

Okay folks, @Mikejmnez @malmans2 , the tests are passing now that we've pinned cartopy<0.20 and I've tested the new environment on Sciserver and ran all the notebooks as well. @Mikejmnez I don't have your LLC4320 notebook, but apart from that all looks great !

@Mikejmnez
Copy link
Collaborator

Mikejmnez commented Feb 17, 2022

You can just test if it is going to work by just creating the od with llc4320:

od = ospy.open_dataset.from_catalog("LLC4320")

if it works, then that is it! The only issue it that intake is able to read multiple zarrs in parallel.

@asiddi24
Copy link
Collaborator Author

asiddi24 commented Feb 17, 2022

I ran my ECCO notebook. It works, but shucks ! It gave me an error with the LLC4320 dataset.

---------------------------------------------------------------------------
OSError                                   Traceback (most recent call last)
File <timed exec>:1, in <module>

File ~/workspace/Storage/asiddi24/persistent/oceanspy/oceanspy/open_oceandataset.py:141, in from_catalog(name, catalog_url)
    138     mtdt = cat[entry].metadata
    140     # Create ds
--> 141     ds = cat[entry].to_dask()
    142 else:
    143     # Pop args and metadata
    144     args = cat[entry].pop("args")

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/intake_xarray/base.py:69, in DataSourceMixin.to_dask(self)
     67 def to_dask(self):
     68     """Return xarray object where variables are dask arrays"""
---> 69     return self.read_chunked()

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/intake_xarray/base.py:44, in DataSourceMixin.read_chunked(self)
     42 def read_chunked(self):
     43     """Return xarray object (which will have chunks)"""
---> 44     self._load_metadata()
     45     return self._ds

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/intake/source/base.py:236, in DataSourceBase._load_metadata(self)
    234 """load metadata only if needed"""
    235 if self._schema is None:
--> 236     self._schema = self._get_schema()
    237     self.dtype = self._schema.dtype
    238     self.shape = self._schema.shape

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/intake_xarray/base.py:18, in DataSourceMixin._get_schema(self)
     15 self.urlpath = self._get_cache(self.urlpath)[0]
     17 if self._ds is None:
---> 18     self._open_dataset()
     20     metadata = {
     21         'dims': dict(self._ds.dims),
     22         'data_vars': {k: list(self._ds[k].coords)
     23                       for k in self._ds.data_vars.keys()},
     24         'coords': tuple(self._ds.coords.keys()),
     25     }
     26     if getattr(self, 'on_server', False):

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/intake_xarray/xzarr.py:44, in ZarrSource._open_dataset(self)
     42     kw.setdefault("backend_kwargs", {})["storage_options"] = self.storage_options
     43 if isinstance(self.urlpath, list) or "*" in self.urlpath:
---> 44     self._ds = xr.open_mfdataset(self.urlpath, **kw)
     45 else:
     46     self._ds = xr.open_dataset(self.urlpath, **kw)

File ~/miniconda3/envs/Oceanography_test/lib/python3.8/site-packages/xarray/backends/api.py:873, in open_mfdataset(paths, chunks, concat_dim, compat, preprocess, engine, data_vars, coords, combine, parallel, join, attrs_file, combine_attrs, **kwargs)
    870     paths = [os.fspath(p) if isinstance(p, os.PathLike) else p for p in paths]
    872 if not paths:
--> 873     raise OSError("no files to open")
    875 if combine == "nested":
    876     if isinstance(concat_dim, (str, DataArray)) or concat_dim is None:

OSError: no files to open

The version of intake-xarray is 0.5.0+19.g77bd753. I tried to bump it to 0.4.1 but that didn't help either. @Mikejmnez what version of intake-xarray were you using ?

@Mikejmnez
Copy link
Collaborator

Mikejmnez commented Feb 17, 2022

@asiddi24 The 0.5.0+19.g77bd753 is the correct one (don't change that). It looks like it didn't find any files. Are you running it from the Poseidon container? The filedb nodes are only accessible from the Poseidon container.

@asiddi24
Copy link
Collaborator Author

Aha ! of course. I was running it all in Ocean Circulation. Works like a charm now ! Yay ! lol, I'm having so much fun with this. Time to sleep now.

Okay, we're all set.

Copy link
Contributor

@malmans2 malmans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few suggestions you can commit directly on GH if make sense to you.
In general, I would install as much packages as possible as we are not updating the image very often.

Additionally, I would

  • Install python 3.9 rather than 3.8. Let's use the best one we support!
  • Install xmitgcm stable release using conda. I tested it in Pin cartopy #218 and all tests passed, but I'm not up to speed with LLC stuff. @Mikejmnez do you see any downside in using v0.5.2?

BTW, can we close #218? Looks like you pinned cartopy here.

ci/environment.yml Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
sciserver_catalogs/environment.yml Outdated Show resolved Hide resolved
sciserver_catalogs/environment.yml Outdated Show resolved Hide resolved
sciserver_catalogs/environment.yml Outdated Show resolved Hide resolved
@Mikejmnez
Copy link
Collaborator

This all sounds good to me. If can, we should use the stable release of xmitgcm. The stuff in llc_rearrange uses only xarray so there shouldn't be any conflict of downsizes there.

@asiddi24
Copy link
Collaborator Author

I'll make these changes and test the environment again today. I'll let you folks know how it goes.

@Mikejmnez
Copy link
Collaborator

BTW, can we close #218? Looks like you pinned cartopy here.

Yes, I think we should close #218 and just incorporate everything related to the new environment into this PR.

asiddi24 and others added 5 commits February 18, 2022 10:04
Co-authored-by: Mattia Almansi <mattia.almansi@noc.ac.uk>
Co-authored-by: Mattia Almansi <mattia.almansi@noc.ac.uk>
Co-authored-by: Mattia Almansi <mattia.almansi@noc.ac.uk>
Co-authored-by: Mattia Almansi <mattia.almansi@noc.ac.uk>
@malmans2
Copy link
Contributor

@asiddi24 Almost good to go. I think you forgot to use the stable release of xmitgcm

@Mikejmnez
Copy link
Collaborator

Mikejmnez commented Feb 18, 2022

Awesome! Lets wait until Ali (successfully) runs the notebooks on Sciserver before merging. @asiddi24 just let us know! Note that this PR also closes #218

@Mikejmnez Mikejmnez mentioned this pull request Feb 23, 2022
@Mikejmnez Mikejmnez merged commit 0f57baf into hainegroup:master Feb 23, 2022
@Mikejmnez Mikejmnez mentioned this pull request Feb 23, 2022
3 tasks
@asiddi24 asiddi24 deleted the new_env branch September 13, 2022 16:13
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.

0.5 <= xesfm <= 0.6.2 yields Attribute Error with Regridder Updating Oceanography image in SciServer
3 participants