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

Preprocessor multimodel fails due to different scalar aux_coords #1606

Closed
tomaslovato opened this issue May 30, 2022 · 9 comments · Fixed by #1934
Closed

Preprocessor multimodel fails due to different scalar aux_coords #1606

tomaslovato opened this issue May 30, 2022 · 9 comments · Fixed by #1934
Assignees
Labels
preprocessor Related to the preprocessor
Milestone

Comments

@tomaslovato
Copy link
Contributor

The creation of a preprocessor to compute global spatial averaged fields on native grids
correctly create 1D cubes but with different lat/lon (or i,j) values.
This trigger an iris exception error when the multimodel preprocessor attempt to merge the cubes

merged_cube = cubes.merge_cube()

As temporary workaround, using data regridding before the spatial average solves the problem,
but I think that the support to work with native grids data derivation is a relevant item for the system

Attachments
test_time_mm.yml
main_log_debug.txt

@tomaslovato tomaslovato added the preprocessor Related to the preprocessor label May 30, 2022
@tomaslovato tomaslovato changed the title Preprocessor multimodel fails with due to different scalar aux_coords Preprocessor multimodel fails due to different scalar aux_coords May 30, 2022
@zklaus
Copy link

zklaus commented May 30, 2022

I agree that native grid support is important. I am not completely sure how to tackle that. Hm. Perhaps we can have a special case that checks whether the entire globe is covered and just put a standardized scalar coordinate in?

@tomaslovato
Copy link
Contributor Author

@zklaus Maybe a check on the coordinates that are reduced to 0D could be used to remove them from the cube ...

@tomaslovato
Copy link
Contributor Author

I made some additional tests on this issue ...

Part 1
I think we can clean certain scalar coordinates after the area average operation, by preserving some relevant ones, like e.g. shape_id , (I picked up the idea from a similar operation made in _multimodel.py#L259-L264 )

My solution was to add the following code to _area.py

diff --git a/esmvalcore/preprocessor/_area.py b/esmvalcore/preprocessor/_area.py
index 8f79e2da4..1b835af71 100644
--- a/esmvalcore/preprocessor/_area.py
+++ b/esmvalcore/preprocessor/_area.py
@@ -309,6 +309,14 @@ def area_statistics(cube, operator):
             "area_statistics changed dtype from "
             "%s to %s, changing back", original_dtype, new_dtype)
         result.data = result.core_data().astype(original_dtype)
+
+    # remove spatial scalar coordinates
+    scalar_coords_to_keep = ['shape_id']
+    for coord in result.aux_coords:
+        dims = result.coord_dims(coord)
+        if not dims and coord.long_name not in scalar_coords_to_keep:
+            result.remove_coord(coord)
+
     return result

Part 2
Along with this I made a test to check if adding a regrid operation before the spatial average could be a viable workaround for the time being ... but I found another bug!
The regrid operation removes all scalar coordinates while if one of the input dataset is on a regular grid it will retain the scalar coordinates leading to the same error when performing the merge_cube() operation.

In this case I think that removing the scalar coordinates from the input dataset that are on a regular grid will be the best option (mainly because scalar coordinates are not available in the regridded cubes).
I added the following piece of code to _regrid.py when dealing for the regular grid case

diff --git a/esmvalcore/preprocessor/_regrid.py b/esmvalcore/preprocessor/_regrid.py
index ad5d8b9b3..fa9f2cfb4 100644
--- a/esmvalcore/preprocessor/_regrid.py
+++ b/esmvalcore/preprocessor/_regrid.py
@@ -653,6 +653,12 @@ def regrid(cube, target_grid, scheme, lat_offset=True, lon_offset=True):
             cube.coord(coord).points = target_grid.coord(coord).points
             cube.coord(coord).bounds = target_grid.coord(coord).bounds

+        # remove scalar coordinates
+        for coord in cube.aux_coords:
+            dims = cube.coord_dims(coord)
+            if not dims:
+                cube.remove_coord(coord)
+
     return cube

@tomaslovato
Copy link
Contributor Author

@zklaus I'm not sure if this issue was somehow solved in the new RC, I'll try in the coming days to rerun the test once that v2.7,0 is marked as stable.

@tomaslovato
Copy link
Contributor Author

I run again the proposed test recipe test_time_mm.yml with today's main code repos (ESMValCore: 2.7.0rc3.dev4+g9dec201c6, ESMValTool: 2.6.1.dev35+g221e222ce ) and the error is still there.

I applied the previously suggested changes and the recipe runs fine.

@tomaslovato
Copy link
Contributor Author

As this issue is silent since a while, I re-run again the initially proposed test recipe test_time_mm.yml with today's main code repos (ESMValCore: 2.8.0.dev77+gd61d200cf, ESMValTool: 2.8.0.dev66+g355c634d1 ) and the error is still there.

I applied the previously suggested changes and the recipe runs fine.

@tomaslovato
Copy link
Contributor Author

@zklaus @bouweandela @remi-kazeroni is there any chance that this issue could be addressed in the coming v2.8?

@schlunma
Copy link
Contributor

Hi @tomaslovato, sorry for the late answer! Yes, we would be happy to include a fix for this in v2.8! Would an optional argument ignore_scalar_coords to the multi_model_statistics preprocessor (which basically removes all scalar coords before merging when set to true) work for you? That way we make sure that no other recipes (which might depend on other scalar coords) are affected by this change.

I can open a pull request this afternoon if you are fine with this!

@tomaslovato
Copy link
Contributor Author

Thanks @schlunma for the reply !!
Your idea of adding the option ignore_scalar_coords looks good ... if you can implement it in a dedicate PR, I'll be happy to contribute with testing and feedbacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preprocessor Related to the preprocessor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants