ESCOMP / CTSM

Community Terrestrial Systems Model (includes the Community Land Model of CESM)
http://www.cesm.ucar.edu/models/cesm2.0/land/
Other
310 stars 315 forks source link

Add an action to test python code through different python versions #1580

Open ekluzek opened 2 years ago

ekluzek commented 2 years ago

We talked about this at the CTSM software meeting this morning. Something that wouldn't be hard to do would be to run the python unit tests in the python directory through different versions of python that we want to support. I would envision this would try with a list of python versions like:

3.7 3.8 3.9 3.10

As new versions are available we can add them to the list. If there's a reason to try micro versions within a major version that could be done as well. If the minimum version required is advanced, older versions could be taken off the list.

ekluzek commented 2 years ago

There's some discussion here

https://github.com/ESCOMP/CTSM/pull/1419#issuecomment-1018975634

in PR #1419 about sensitivity that likely has to do with python versions (or python environment or user environment). This makes me think that possibly this issue should be elevated, so we get more information on these type of issues.

ekluzek commented 2 years ago

Note, one interesting thing I noticed with this is that when I test with different versions of python on cheyenne (using conda) pylint can show errors in one version, but not others.

ekluzek commented 2 years ago

The bit longer term solution for the pylint issues will be that @negin513 will setup some conda environments that we can use. We should have a few different python versions for those test environments as well.

In the short term I think the standard should be to do this on cheyenne:

module load python. # Which gives you the default python version, currently 3.7.9) ncar_pylib

@negin513 is going to teach us about python environments and also get the environments setup in a future PR. This part of this issue should be a separate issue.

negin513 commented 2 years ago

Similar issue also came up in the discussion of PR#1419: @slevisconsulting comment about this : https://github.com/ESCOMP/CTSM/pull/1419#issuecomment-1013772176 My response on what possibly causing this issue: https://github.com/ESCOMP/CTSM/pull/1419#issuecomment-1018975634

ekluzek commented 2 years ago

I have a conda environment file that makes some progress here for ctsm5.1.dev100. It looks like merely moving the python version ahead doesn't cause it to break. And numpy and xarray can go to the current latest version for example. Using a 3.10 version of python is a problem though, and pylint must be carefully controlled or else it shows errors.

But, for example the following requirements file does work:

python>=3.9.13,<3.10    # Moving to 3.10 runs into conflicts
scipy
netcdf4
requests
numpy>=1.23.0
xarray>=2022.3.0
pylint>=2.8.3,<2.9.0  # Once, you move off of 2.8.3, make lint shows 2 errors, at 2.11.1 there are 2 more errors beyond that
                      # By, 2.14.4 there are 75 errors
black>=22.6.0

This suggests to me that either we don't have enough testing for our python code, or that truly using the latest version just before python 3.10 is fine.

I'm also concerned about the need for the pylint version to be carefully controlled. I think we should fix some of these issues that arise in later versions, and start a later version.

ekluzek commented 2 years ago

In ctsm5.1.dev100 I added a file called conda_env_ctsm_py_latest.tx that can be used with manage_python_env to test the latest working python environment. I think this is what would be good to have an action to run the testing with. I expect people to use the standard environment for tags that are made, having the latest working environment tested automatically sounds like a good use of github actions to me. I think this would be simple to do as well. It could be setup to happen on push so it would show you an error if you committed something that didn't pass.

@negin513 and @billsacks any thoughts on doing that?

billsacks commented 2 years ago

I will defer to others on this, since I don't have experience with managing different python environments.

ekluzek commented 1 year ago

I've done some work on this so that our py_env_create tool has a few different options for how you create python environments. One of the environments is for a "latest" environment that uses explicit latest versions of packages.

I also did some work on getting github actions to work for automatic testing of these things through github actions.

ekluzek commented 1 week ago

This was discussed at CSEG today. Endorsing doing this sort of thing.

cime is testing through python versions with updates. But, components will need to do this.

https://docs.google.com/document/d/186U6-dt_wWZZGU9NzYQ5zNlMnpx9XX6oweuTXzQY-oo

negin513 commented 1 week ago

I'm glad to hear that other CESM components are increasingly adopting third-party packages and plan to use this approach!

One thing to keep in mind is that if all components specify their own dependencies with detailed version requirements, it could lead to clashes in the Conda environments. To avoid this, I’d recommend specifying minimal version requirements wherever possible. This gives the package manager more flexibility to resolve conflicts. A simple CI/CD workflow can test to make sure there are no environment conflicts between components and it does not need to run on Derecho (but it can). I got a notification on this since I was involved in this discussion back then, but LMK if I can help with this.

P.S. This brings back memories—IIRC, we discussed the topic of adopting 3rd party packages during the first CSEG meetings I joined back in 2019. IMHO, avoiding third party packages in Python ecosystem is almost inevitable.