Skip to content

Convert geometric_features to python package with python 3 support#102

Merged
xylar merged 23 commits intoMPAS-Dev:masterfrom
xylar:convert_to_package
May 8, 2019
Merged

Convert geometric_features to python package with python 3 support#102
xylar merged 23 commits intoMPAS-Dev:masterfrom
xylar:convert_to_package

Conversation

@xylar
Copy link
Collaborator

@xylar xylar commented Feb 24, 2019

This PR represents my proposal for a rather big change to geometric_features. It includes:

  • Support for python 3
  • Moving command-line scripts into entry points and functions in a python package, accessible from within python scripts
  • A more intuitive (I believe) and more extensible interface for manipulating feature collections
  • Moving the geometry data into a subfolder geometry_data for clarity and separation from the code
  • Moving driver_scripts to examples (since they rarely get used as "drivers")
  • The capability to download geometry from the GitHub repo on the fly as needed
  • Addition of a recipe for building a conda package

The existing example scripts, the README.md, the one unit test we currently have, and CI have all been updated to work with the new interface.

The purpose of this work is:

  • to make geometric features easier to use from elsewhere (e.g. COMPASS)
  • to get ready for the pending demise of python 2.7
  • to clean up what had become an increasingly difficult interface to work with and develop

closes #101
closes #96

@pep8speaks

This comment has been minimized.

@xylar xylar force-pushed the convert_to_package branch from b5c11b2 to 82db0ca Compare February 24, 2019 09:04
@xylar
Copy link
Collaborator Author

xylar commented Feb 24, 2019

I have a corresponding branch here: xylar/MPAS-Model/update_geometric_features_api where I have updated the new global_ocean test cases (but not global_ocean_V1) in COMPASS to work with the new API. This script shows most of the changes:
https://github.com/xylar/MPAS-Model/blob/ocean/update_geometric_features_api/testing_and_setup/compass/ocean/global_ocean/cull_mesh.py

I tested it out and it should work once geometric_features is available as a conda package.

@xylar
Copy link
Collaborator Author

xylar commented Feb 26, 2019

To test the conda package, you can do:

conda create -n compass -c conda-forge -c xylar python=3.7 geometric_features mpas_mesh_tools
conda activate compass

(You could use python 2.7 if you prefer). You should then be able to download one of the example scripts anywhere you like and run it without having to checkout the geometric_features repo at all.

xylar added 15 commits March 6, 2019 19:03
More is needed because of python 3
The same should apply here, since this was developed for MPAS-Model
and is used in the COMPASS infrastructure of MPAS-Model to create
test cases.
This new package has all the functionality of the top-level scripts
and helpers but requires less code to do it and can be called
from within other python code.
This keeps the reference data more clearly separate from other
contents of the repository.
These are replaced by the new package
With the new infrastructure, these will hopefully not be used
directly in anyone's workflow.
The default branch to check out geometric features from is the
tag matching the version number of the code.
@xylar xylar force-pushed the convert_to_package branch from 82db0ca to a6e9204 Compare March 7, 2019 02:19
@xylar xylar changed the title [WIP] Convert geometric_features to python package Convert geometric_features to python package Mar 7, 2019
@xylar
Copy link
Collaborator Author

xylar commented Apr 24, 2019

Run a nightly regression suite pointing to this branch of geometric features - I can do that.

@mark-petersen, This will not work without changes to COMPASS, as indicated above (#102 (comment)). I don't think we intend to modify the existing global ocean test cases to work with this version of geometric features. So this would require developing a new test suite for the non-V1 test cases. Essentially, this is a first step toward dropping v1 test cases from COMPASS and moving in a new, more coordinated direction. Maybe you and I should have a phone call about this. I'm not sure I have time today, though.

@xylar
Copy link
Collaborator Author

xylar commented Apr 24, 2019

Thanks, @milenaveneziani. Adding some new regions would indeed be a good way to test this. No big rush, just wanted to make sure this doesn't get abandoned entirely.

@mark-petersen
Copy link
Collaborator

I see. But I could test a single global_ocean case (not a suite) with your branch
xylar/MPAS-Model/update_geometric_features_api
pointing to this branch of geometric features. Is that right?

@xylar
Copy link
Collaborator Author

xylar commented Apr 24, 2019

@mark-petersen, close. You could test the xylar/MPAS-Model/update_geometric_features_api after creating a new conda environment as described in #102 (comment). The whole point off this work is that you would no longer point to a checkout of the geometric_features repo, you would just point to an empty (or previously populated) cache directory and only the geometry you need would be downloaded locally. The package for managing geometric features is a conda package installed in your conda environment and the features themselves are pulled in from GitHub as needed.

If you're testing this on IC, it's slightly painful to install Miniconda so you can create your own python environments (because of the firewall) but I could work you through it.

@mark-petersen
Copy link
Collaborator

@xylar so far so good - on IC in my own conda directory, I was able to use your commands in #102 (comment) to create and activate the compass conda environment.

Could you rebase xylar/MPAS-Model/update_geometric_features_api onto ocean/develop? There are some conflicts with that compass update.

Using python 3, it still gets stuck on print statements. Using python 2 it hangs at

./paraview_vtk_field_extractor.py
  File "/turquoise/usr/projects/climate/mpeterse/repos/tools/master/visualization/paraview_vtk_field_extractor/utils.py", line 16, in <module>
    from pyevtk.vtk import VtkFile, VtkPolyData
ImportError: No module named pyevtk.vtk

I'm not sure if that needs to be added to the environment, or done a different way in MPAS-Tools.

Commenting out the vtk line, I was able to get

global_ocean/QU240/init/base_mesh

to work completely. I then tried culled_mesh but on IC it says:

  natural_earth/region/Land_Coverage/region.geojson could not be reached!

on a compute node. On a log-in node, it is

ERROR while downloading natural_earth/region/Land_Coverage/region.geojson:
404 Client Error: Not Found for url: https://raw.githubusercontent.com/MPAS-Dev/geometric_features/0.1/natural_earth/region/Land_Coverage/region.geojson
...
IOError: [Errno 2] No such file or directory: u'geometric_data/natural_earth/region/Land_Coverage/region.geojson'

Is that a directory from this repo. So I used git to download this repo by hand and made a soft link. It runs cull_mesh.py until

running ./widen_transect_edge_masks.py -f critical_passages_mask.nc -m base_mesh.nc -l 43.0
Traceback (most recent call last):
  File "./widen_transect_edge_masks.py", line 40, in <module>
    nTransects = len(maskFile.dimensions["nTransects"])
KeyError: 'nTransects'

which is linked to the current head of MPAS-Tools. Is that supposed to be replaced with an internal python call?

@xylar
Copy link
Collaborator Author

xylar commented Apr 24, 2019

@mark-petersen, thanks for all that testing! It looks like a lot has happened in 2 months that means I need to clean things up. I'll do that and ask you to give things another try soon. Sorry you struggled but this is all really helpful to me!

@mark-petersen
Copy link
Collaborator

@xylar no problem at all. I only spent 15 minutes, and a little problem-solving is good for me.

@mark-petersen
Copy link
Collaborator

@sbrus89, @xylar is making a python package for COMPASS. What special python libraries are you using for your coastal setup? We should include them here.

LIGHT uses pyamg in COMPASS setups, as noted MPAS-Dev/MPAS-Model#56 (comment)

@pwolfram
Copy link
Contributor

@mark-petersen, @sbrus89 is on vacation-- I know he is using pyflann for instance. But, there are probably others too.

@xylar
Copy link
Collaborator Author

xylar commented Apr 25, 2019

Don't worry too much. I'll figure these out as I test.

@xylar xylar force-pushed the convert_to_package branch from cde9dee to 68e9bee Compare April 28, 2019 14:53
@xylar
Copy link
Collaborator Author

xylar commented Apr 28, 2019

@mark-petersen, I've made some headway on getting things back up and working. However, I discovered this issue in the process:
dengwirda/jigsaw-geo-matlab#26
Testing will require a workaround until that's fixed, see below.

I also found some incompatibilities with python 3 in the COMPASS framework, see MPAS-Dev/MPAS-Model#222
and some issues with python 3 in CVMix (now fixed in master, see CVMix/CVMix-src#76) and Ocean-BGC (see E3SM-Project/Ocean-BGC#1).

Please create a new conda environment like this:

conda create -n compass_py3.7 -c conda-forge -c xylar python=3.7 geometric_features mpas_tools \
    jigsaw metis pyflann scikit-image basemap
conda activate compass_py3.7

Please clone jigsaw-geo-matlab to a new location (I know you have it already but the point is to pretend you're a new user).
Here's the temporary fix:

cd jigsaw-geo-matlab/jigsaw/bin/LNX-64/
which jigsaw
ln -sfn  <path/to/jigsaw> jigsaw64r

where <path/to/jigsaw> is the location of the jigsaw executable in your conda environment, something like /home/xylar/miniconda3/envs/compass_py3.7/bin/jigsaw

Checkout my branch xylar/MPAS-Model/ocean/update_compass_api_geom_feat_and_mesh_tools. Cherry-pick these 2 commits from my framework commit: 093d7fa708a7e8857275d4d47d28f221b92d8e27 and 834b3b8e6ebfd8b73b0c034c867a483d91c91281. Do what you would normally do to build the model (this may require deactivating the compass_py3.7 environment, in which case you should reactivate it after the model is built). Try building the global_ocean/QU240/init test case and other test cases from the nightly regression suite (besides those from global_ocean_V1, which we agreed would be removed in the near future). All these tests worked for me with the python 3 env. and the fixes above. I hope that's the case for you, too.

My config.ocean looks like this:

...
[executables]
model = /home/xylar/code/mpas-work/model/ocean/update_compass_api_geom_feat_and_mesh_tools/ocean_model
[paths]
mpas_model = /home/xylar/code/mpas-work/model/ocean/update_compass_api_geom_feat_and_mesh_tools
mesh_database = /home/xylar/data/mpas/meshes
initial_condition_database = /home/xylar/data/mpas/initial_conditions
geometric_data = /home/xylar/data/mpas/geometric_data
jigsaw-geo-matlab = /home/xylar/code/jigsaw-geo-matlab/jigsaw-geo-matlab
bathymetry_database = FULL_PATH_TO_BATHYMETRY_DATABASE
jigsaw_to_MPAS = /home/xylar/code/mpas-work/model/ocean/update_compass_api_geom_feat_and_mesh_tools/testing_and_setup/compass/ocean/jigsaw_to_MPAS

A goal is to eliminate as any executables and paths as possible, but particularly all references to geometric_features (but not geometric_data) and to mpas_tools, since these scripts should be part of the conda environment instead.

Other test cases outside of the nightly regression suite also need attention, so we're not ready to merge yet but I want to make sure you can get this far.

I also tested the QU240 init test case with python 2.7, so you can try as above but changing 3.7 to 2.7, but the priority is making sure python 3 is supported. (You can use the same jigsaw-geo-matlap if you don't want to repeat that step but it'll use jigsaw from the other conda environment. That's not a problem because it doesn't use python so the python version is irrelevant.)

@xylar
Copy link
Collaborator Author

xylar commented Apr 30, 2019

So the feature where this package downloads the geometric_data as needed is causing trouble because the compute nodes (at least on LANL IC) don't have access to GitHub. We could pre-store the geometric data somewhere (probably a fine solution) but there's a risk that the stored geometric_data isn't for the right version of geometric_features so we'll need to take care.

@xylar
Copy link
Collaborator Author

xylar commented May 7, 2019

@mark-petersen, the temporary fix (mentioned above) to jigsaw-geo-matlab should no longer be necessary if you update to the latest version.

Copy link
Collaborator

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

Tested compass_py3.7 package and this PR and successfully on grizzly and badger, compute node and front end node (except mpirun calls). Tested QU240 init, QU240 test, EC60to30 init.

@xylar
Copy link
Collaborator Author

xylar commented May 8, 2019

Great! I'm going to remove a commit where I point to a temporary branch and then merge this PR.

@xylar xylar force-pushed the convert_to_package branch from 4bd5ba4 to 28e85ea Compare May 8, 2019 15:04
@xylar xylar merged commit 7f15315 into MPAS-Dev:master May 8, 2019
@xylar xylar deleted the convert_to_package branch May 8, 2019 15:08
@xylar xylar mentioned this pull request May 8, 2019
mark-petersen added a commit to MPAS-Dev/MPAS-Model that referenced this pull request May 10, 2019
…s' into ocean/develop

A large number of updates to the COMPASS ocean API including:

- using the geometric_features conda package
  (MPAS-Dev/geometric_features#102)
- using the mpas_tools conda package (MPAS-Dev/MPAS-Tools#248)
- updating the jigsaw_to_MPAS scripts to work as a python package and to
  support python 3
- update global_ocean scripts to python 3 and to use these new packages

NOTE: This merge removes the capability to create the global meshes used
in E3SM version 1, (2017-2018), where the base meshes were created with
Doug Jacobsen’s tool.  The global mesh creation remaining after this
commit only uses the Jigsaw tool.

In order to recreate meshes and initial conditions used in E3SM version
1, use repositories as of
March 1, 2019:
1. MPAS-Model commit:42029f218175d893cf25e19a26d9b43382a44d54
2. MPAS-Tools commit:6d0fa3a24e879a1a5b6149d9fbfe80fa021489dc
3. geometric_features tag:E3SMv3grids 76709be

closes #220
closes #213
closes #132
closes #166
ashwathsv pushed a commit to ashwathsv/MPAS-Model that referenced this pull request Jul 21, 2020
…esh_tools' into ocean/develop

A large number of updates to the COMPASS ocean API including:

- using the geometric_features conda package
  (MPAS-Dev/geometric_features#102)
- using the mpas_tools conda package (MPAS-Dev/MPAS-Tools#248)
- updating the jigsaw_to_MPAS scripts to work as a python package and to
  support python 3
- update global_ocean scripts to python 3 and to use these new packages

NOTE: This merge removes the capability to create the global meshes used
in E3SM version 1, (2017-2018), where the base meshes were created with
Doug Jacobsen’s tool.  The global mesh creation remaining after this
commit only uses the Jigsaw tool.

In order to recreate meshes and initial conditions used in E3SM version
1, use repositories as of
March 1, 2019:
1. MPAS-Model commit:42029f218175d893cf25e19a26d9b43382a44d54
2. MPAS-Tools commit:6d0fa3a24e879a1a5b6149d9fbfe80fa021489dc
3. geometric_features tag:E3SMv3grids 76709be

closes MPAS-Dev#220
closes MPAS-Dev#213
closes MPAS-Dev#132
closes MPAS-Dev#166
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for python 3 Provenance producing git errors when not in a valid git repository

5 participants