Switch to Iris 3 #819
Switch to Iris 3 #819
Conversation
Use new attribute .metadata instead
|
|
It seems that all cases of Remaining failing tests:
|
@schlunma Would you be able to help out with this? |
This shouldn't be hard to fix. Just add a |
@schlunma Thanks for the hint! I managed to track down a few of the bugs related to that! |
|
I updated |
|
The remaining tests: fail with: for example: I traced this back to |
|
Sorry, if |
|
Is this related to ancillary variables? If so, it seems like they did some work there, see the third feature here: https://scitools-iris.readthedocs.io/en/v3.0.0rc0/whatsnew/3.0.html#features |
|
No, I don't think so. From this documentation it seems to me that the handling of derived coords has not change, because |
|
Yeah I already checked between master and this PR. The difference is as I described above. On master there is a |
|
Pretty sure the problem is with how the dataset is constructed. For example, this one here: This is what happens if I load the same data with the 2 different version of iris. import iris
cubes = iris.load('cl_a.nc')iris2iris3 |
|
Or is this simply the same |
|
I managed to fix one of them by setting the correct units ('1') |
|
All tests are working again 🥳 |
|
CircleCI is failing as the code requires |
|
I found an issue with the concatenation of overlapping cubes (see here). On DKRZ, we have these two files for With the current The stack trace of the log file is considerably smaller, so most of the error is only written to stdout: This is the recipe I'm using: If necessary I can provide the two involved datasets. |
|
@schlunma It looks like the cube you constructed breaks when you call |
yeah but when you do it, say you are not from ESMValTool 😆 One thing to be careful with is that you are actually running with Python 3.7 - both the current esmvalcore and the iris3 branch of ESMValTool (which I assume you are using) is running under Python 3.9 naturally adopted by conda (if you use esmvalcore in pure development mode, as you should since you are testing this branch not the released version) - it could be an issue with the Python version, that the iris guys have not spotted since I think they've run all their tests with Python 3.9 |
Yeah, I thought that it's something related to that...I have a detailed look at this tomorrow, it's probably also connected to our custom aux factory |
|
I have recreated the conda env with python=3.7 and all tests pass fine, I tried this wee script with the files in import iris
import logging
logger = logging.getLogger(__name__)
print(iris.__version__)
c = iris.load("tests/integration/cmor/_fixes/test_data/common_cl_ap.nc")
for ci in c:
logger.info("cube %s", str(ci))(yes, I have just saved you 2 minutes from writing it yourself 😆 ) |
These were caused by your local cache, the tests passed fine on CircleCI. I tried to make the cache key a bit more specific so this doesn't happen again (hopefully) and restored the original netcdf files. |
I'd be happy to approve, or should we wait fo solve the latest issue reported by @schlunma? |
cheers Bouwe, let's wait for Manu see what's the deal with that roadblock |
|
Ok, I found the issue, but I have no clue why my fix works. The issue is the function I could fix the issues by replacing these lines ESMValCore/esmvalcore/cmor/_fixes/shared.py Lines 39 to 43 in b632f9a with a self.units = pressure_at_top.unitsin the class constructor, similar to the So apparently a I continue my tests now (will check a couple of recipes which I now may be problematic sometime, so stay tuned). |
|
Bad news, I'm getting quite a lot of errors with several recipes:
I'm pretty sure these are not the only failures. I think before merging this and including it in the new release we really need to test all recipes, otherwise I'm pretty sure that some won't work in the future. |
yip, the |
many thanks for testing all this @schlunma - top job indeed! Do you reccomend halting until further tests be done? I think we can do this: we freeze the code on Monday for the release but keep working on making sure iris3 is indeed secure to be included in the release and act upon what I propose in #963 as sort of an emergency commit before release, if we manage to make it work by then, that is |
|
@valeriupredoi I managed to fixed all the issues I found. With these all the recipes I tested (some of mine and all example recipes where I have the data) run now. Should I push them to this branch? I think merging this is found as long as we thoroughly test all recipes before the release. |
great! given the tests pass fine here, I really hope there will be no more changes the Core needs to accommodate |
|
I tested all variable derivations, lots of preprocessors and lots of variables, but we cannot be sure that all recipes work fine now. I hope that they do 😬 And if they don't work, I hope that only minor changes are necessary to get them working again... |
There was a problem hiding this comment.
Thanks for the great team work and lots of last-minute effort everyone, let's get this in!
There's bound to be a few remaining problems, especially in the tool, but we still have 3 weeks before that will be released, so that should be enough time to find and fix everything.
If anyone would still like to use the development version of the tool with iris 2, I have created an iris2 branch that we can keep around for the next month. If you still need an old development version after that, you can go back in history from the master branch by running git log, selecting the commit hash (the long hexadecimal number after the word 'commit' in the log) and running e.g. git checkout 1c9885bb9796c88f3a14e21ef4148968fe9475b4. Of course, it is also possible to install older versions from Conda.
|
excellent team work, guys! Props to @schlunma @stefsmeets @jvegasbsc and @bouweandela and of course to @bjlittle and the iris team that have released the fastest bugfix release in the history of iris 😁 |
Upgrade to iris 3.
Related ESMValTool pull request: ESMValGroup/ESMValTool#1874
Test results
Automated Github Actions tests with
iris3and changes to esmvalcore tests from this PRiris3and known to usCircleCI tests
Conda build fails because of the standard Circle build issue if adding the
--channelpointer to the rc label channel (not relevant, GA conda build works well), test suite fails because the env is prebuilt in the docker container with stock esmvalcore=2.0(iris 2.4) so all OK 👍To do list
We should keep this PR as is until
iris3becomes available onconda-forgeand PyPi. Using the label channelconda-forge/label/rc_irisworks well, as seen from the tests above, but needs to be kept as testing only in this PR; so before merging into master, we need to revert changes to (and change accordingly for upper limits of iris version):conda-forge/label/rc_iris'scitools-iris=3.0.0'-c conda-forge/label/rc_irisonce iris 3.0.0 hitsconda-forgeThis PR tracks the progress to use iris3. I'm going by the tests to fix the issues. Some of them are already listed in #376
I will add the current test output in the next comment.
Tasks
If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.
Closes #378
The main issues are that:
CubeList.extract_stricthas been deprecated, useCubeList.extract_cubeinstead.iris3apparently won't deal with dimensionless coordinates anymore, that means that coordinates must have their units set, i.e.some_coord.units = '1'orAuxCoord(..., units=Unit('1').