Skip to content

Implemented regridding of auxiliary coordinates #548

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

Closed
wants to merge 6 commits into from

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Mar 4, 2020

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • This pull request has a descriptive title that can be used in a changelog
  • Add unit tests
  • Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
  • If writing a new/modified preprocessor function, please update the documentation
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes
  • If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

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 #545.

@schlunma schlunma added enhancement New feature or request iris Related to the Iris package labels Mar 4, 2020
@schlunma schlunma self-assigned this Mar 4, 2020
@valeriupredoi
Copy link
Contributor

may be out of context but here's something that may be of use - was just sifting through iris issues waiting for a du -h BIG_DIR to finish SciTools/iris#3653

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

is there no way to add bounds to aux coords in iris so that we can jump over the constraint that is there be no bounds there be no regridding? In a peasant way you can promote that coord to DimCoord and use guess_bounds()

@valeriupredoi

This comment has been minimized.

@schlunma

This comment has been minimized.

@schlunma

This comment has been minimized.

@valeriupredoi

This comment has been minimized.

@valeriupredoi

This comment has been minimized.

@schlunma

This comment has been minimized.

@schlunma

This comment has been minimized.

@valeriupredoi

This comment has been minimized.

@valeriupredoi

This comment has been minimized.

@valeriupredoi

This comment has been minimized.

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

if you put that logger message saying the regrid method for AuxCoords is fixed to BLAH, all good by me bro, let's get @zklaus take on the actual philosophy 😁

@schlunma schlunma changed the title Implemented regridding of multi-dimensional auxiliary coordinates Implemented regridding of auxiliary coordinates Mar 5, 2020
@schlunma schlunma added this to the ESMValTool papers milestone Mar 10, 2020
@valeriupredoi
Copy link
Contributor

waddup, guys? 🍺

@schlunma
Copy link
Contributor Author

waddup, guys? beer

I think @zklaus had some comments on this, right?

@zklaus
Copy link

zklaus commented Mar 13, 2020

I am not sure regridding is generally applicable to aux coords. I'd like to see the actual use case that triggered that to get a better idea. @schlunma, could you mention the issue of the use case here (again?)?

@schlunma
Copy link
Contributor Author

Sure thing. Some FGOALS model data has a derived coordinate atmosphere_sigma_coordinate which cannot be read by iris automatically (see #564). Since we save nc files at then end of the preprocessing and load them again in the diagnostic, adding an aux_factory to those files during the preprocessing step is useless, since at loading step iris does not automatically know about this aux_factory and we have to add it manually again in the diagnostic.

For this, it is necessary that the file contains the surface_pressure coordinate (which is an aux coord). Without regridding, this works fine, but as mentioned, regridding drops this coordinate.

The funny thing about that is that iris does not drop these coordinates if they belong to one of the aux factories that iris supports automatically. Otherwise, every aux coord is lost after iris.cube.Cube.regrid.

Copy link

@zklaus zklaus left a comment

Choose a reason for hiding this comment

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

I am still a bit queasy about applying this approach to all aux coords. I would suggest to limit it either to known good aux coords, like surface_pressure, or at least to those that are connected to the x and/or y dim of the cube.

Comment on lines +210 to +212
points = iris.util.broadcast_to_shape(coord.core_points(),
cube.shape,
coord_dims)
Copy link

Choose a reason for hiding this comment

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

Do I understand correctly that this has the potential to incur a hefty additional cost? A 1d aux coord, even a scalar coord might be blown into a long timeseries of 3d data, right? If I understand iris correctly, this is not necessary. Source and target only need to share x and y dim. What happens without the broadcast?

@schlunma
Copy link
Contributor Author

Okay, I justed tested again with the FGOALS model and I realized that adding atmosphere_sigma_coordinate to #546 solves the problem without this fix. I completely forgot about this, sorry!

I'm really sorry for wasting your time with this PR @valeriupredoi @zklaus!

@schlunma schlunma closed this Mar 13, 2020
@schlunma schlunma deleted the regrid_multidim_auxcoords branch March 13, 2020 15:27
@zklaus
Copy link

zklaus commented Mar 13, 2020

Don't worry. Might still be an interesting concept in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request iris Related to the Iris package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cubes lose auxiliary coordinates during regridding
3 participants