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

Allow exemptions to coordinate axis guessing on save #5003

Closed
bayliffe opened this issue Sep 29, 2022 · 10 comments · Fixed by #5551
Closed

Allow exemptions to coordinate axis guessing on save #5003

bayliffe opened this issue Sep 29, 2022 · 10 comments · Fixed by #5551
Assignees
Labels
Dragon 🐉 https://github.com/orgs/SciTools/projects/19?pane=info Feature: NetCDF + CF-conventions Type: Bug
Milestone

Comments

@bayliffe
Copy link
Contributor

Within the iris NetCDF saver there is a call to guess_coord_axis. This simple utility relies on coordinate names, units, or other attributes to ascribe an axis identifier to each dimension coordinate where possible. One of these guesses checks whether the units of the coordinate are convertible to hectopascals, and if so presumes the dimension is a pressure coordinate corresponding to vertical levels; an axis identifier of Z is added.

Working with probabilistic data in which a dimension coordinate may describe different pressure thresholds, i.e. probability of pressure at mean sea level exceeding 1000hPa, 1010hPa, 1020hPa etc. this results in the threshold coordinate being identified as a vertical spatial dimension with an axis identifier of Z. This is undesirable and causes issues downstream of where the data is created (e.g. in Visual Weather). As this is added on save it is not possible to remove the attribute without using an alternative tool to Iris.

This issue requests that there be a means within Iris of preventing the axis identifiers being added to specific named (or var named) coordinates on save.

Example cube

Below is shown a simple print out of a probabilistic PMSL forecast with a threshold coordinate named air_pressure_at_sea_level that has units of Pa, which is not a height coordinate, but which is assigned a Z axis identifier.

probability_of_air_pressure_at_sea_level_above_threshold / (1) (air_pressure_at_sea_level: 85; projection_y_coordinate: 970; projection_x_coordinate: 1042)
     Dimension coordinates:
          air_pressure_at_sea_level                                                      x                            -                             -
          projection_y_coordinate                                                        -                            x                             -
          projection_x_coordinate                                                        -                            -                             x
     Scalar coordinates:
          ...

ncdump header information corresponding to this coordinate:

float threshold(threshold) ;
	threshold:axis = "Z" ;
	threshold:units = "Pa" ;
	threshold:standard_name = "air_pressure_at_sea_level" ;
@bjlittle
Copy link
Member

bjlittle commented Oct 5, 2022

@bayliffe Thanks for raising this issue, much appreciated. We think this is an issue that needs addressed 👍

Are you happy to take this issue on yourself with some hand holding?

If you don't have time just let us know and we can move this issue forwards for you.

@bjlittle
Copy link
Member

bjlittle commented Oct 5, 2022

After a wee bit of developer discussion your issues raises questions how the best mechanism to best control the behaviour that you want... so this may require more thought on our behalf.

@bjlittle
Copy link
Member

bjlittle commented Oct 6, 2022

@bayliffe Hmmm there seems to be a few ways to approach this problem in a generic useful way, and TBH this is going to need some considered thought (sorry, no avoiding that).

At this point, I suggest that you up-vote your own issue (and get other like minded users/devs to do so too 😉) to make sure that the priority of this issue bubbles to the top of tasks that require to be addressed by @SciTools/iris-devs 👍

Hope that's okay for now?

@tjtg
Copy link

tjtg commented Oct 7, 2022

I've upvoted as requested - this issue affects our usage of Iris-produced NetCDF with downstream systems (including Visual Weather) at BOM.

I'd argue that this issue should be labelled as a bug rather than an enhancement.
Metadata for probabilistic pressures will always be marked (incorrectly) as a vertical/Z axis. I'm also not aware of any way to work around this issue on the user side, besides editing the saved NetCDF file with a non-Iris tool like netcdf4-python or NCO.

@rcomer
Copy link
Member

rcomer commented Oct 7, 2022

That’s good insight @tjtg. I added the “feature” label, as any mechanism to opt out of current behaviour would be a new feature. However, it never makes sense to think that “air_pressure_at_sea_level” could describe height. If we could change guess_coord_axis to produce a sensible return value for this coordinate, then we would not need an opt-out mechanism.

@ESadek-MO
Copy link
Contributor

Discussed in Peloton, added to 3.5 for time being.

@ESadek-MO ESadek-MO moved this to 🆕 New in 🐻 Iris v3.5.0 Dec 2, 2022
@ESadek-MO ESadek-MO moved this from 🆕Potential Tickets to 📋 Backlog in 🐻 Iris v3.5.0 Dec 2, 2022
@ESadek-MO ESadek-MO moved this from 📋 Backlog to 🆕Potential Tickets in 🐻 Iris v3.5.0 Dec 2, 2022
@ESadek-MO ESadek-MO moved this from 🆕Potential Tickets to 📋 Backlog in 🐻 Iris v3.5.0 Dec 2, 2022
@ESadek-MO ESadek-MO moved this from 📋 Backlog to 🆕Potential Tickets in 🐻 Iris v3.5.0 Dec 2, 2022
@bjlittle bjlittle moved this to 🆕 New in 🦥 Sprint: May 2023 May 2, 2023
@bjlittle bjlittle moved this from 🆕 New to 📋 Backlog in 🦥 Sprint: May 2023 May 12, 2023
@trexfeathers trexfeathers added the Dragon 🐉 https://github.com/orgs/SciTools/projects/19?pane=info label Jul 4, 2023
@trexfeathers trexfeathers moved this to 📌 Prioritised in 🐉 Dragon Taming Jul 10, 2023
@trexfeathers trexfeathers moved this from 📌 Prioritised to 🛡 Championed in 🐉 Dragon Taming Jul 10, 2023
@trexfeathers
Copy link
Contributor

Had a design discussion with @HGWright recently.

None of the below options are without compromise. The current preferred option is a new DimCoord member: space_time.

Tagging all voters: @rcomer @bjlittle @pp-mo @bayliffe @neilCrosswaite @bjwheltor @s-boardman @BelligerG @MoseleyS @PaulAbernethy @simonjackson900 @tjtg @anja-bom @nivnac @thbom001 @btrotta-bom @dmentipl @ivorblockley @benowen-bom @cpelley @gavinevans

Adjust guess_coord_axis()

Don't use HPa for z-guessing

  • ➕: avoids all mistakes like this
  • ➖: makes Iris less active in adding CF-compliant information
  • ➖: likely break existing workflows that are currently fine

Exclude particular names from z-guessing

Suggestion is to exclude anything that includes *pressure_at*.

  • ➕: keeps utility of current z-guessing, but with fewer mistakes
  • ➖: more difficult to understand/predict
  • ➖: hard to be exhaustive with the exclusion list
    Even likely to change over time
  • ➖: very reliant on outside expertise for this, and each user has different cases that they need to work

Adjust saving

Don't alter an existing axis attribute

Intention is that the user will set an appropriate attribute before saving e.g. "NA".

  • ➕: a simple addition with few breakages likely
  • ➖: requires manual user intervention
  • ➖: don't know how downstream readers might interpret this (only T, Z, Y, X are covered in CF)
  • ➖: difficult to inform users that they should do this - no obvious place in the docs

A new kwarg in the NetCDF saver

  • ➕: a visible feature, easy to spot in the documentation
  • ➕: non-breaking addition
  • ➖: requires manual user intervention
  • ➖: the saving API is already too busy
  • ➖: this is pretty specific for the top-level saver, and sets a precedent for adding lots of other 'modes'

A new DimCoord member: space_time

Boolean, default None. Set to True when coord has an axis attribute of T/Z/Y/X. Set to False if a different axis attribute value is present.

Axis guessing during save is not performed if my_coord.space_time is False (since the guessing was only ever intended to add the spatiotemporal axis labels).

Intention is that the user will set my_coord.space_time=False before saving.

  • ➕: a visible feature, easy to spot in the documentation
  • ➕: precedent in the circular and climatological flags - information to determine Iris behaviour
  • ➕: potential opportunities for future use in distinguishing experimental repeats, pertubations etc.
  • ➕: non-breaking addition
  • ➖: requires manual user intervention
  • ➖: fairly significant API addition

@trexfeathers trexfeathers removed their assignment Jul 28, 2023
@trexfeathers
Copy link
Contributor

Team conclusion:

A Coord property: guess_axis. Defaults to True. User can set to False to 'opt-out'. This means that this coordinate always gets the result None when run through guess_coord_axis().

Using a property provides a space for documenting it, too.

We know this affects NetCDF save, plotting, merging, maybe regridding.

@trexfeathers trexfeathers moved this from 🛡 Championed to 🦎 Tamed in 🐉 Dragon Taming Sep 15, 2023
@trexfeathers
Copy link
Contributor

Not currently planning a context manager, but can be persuaded 😉🎂

@stephenworsley stephenworsley moved this to 🆕 New - potential tasks in 🐙Iris v3.8.0 Sep 28, 2023
@stephenworsley stephenworsley added this to the v3.8 milestone Sep 28, 2023
@stephenworsley stephenworsley moved this from 🆕 New - potential tasks to Candidate for next sprint in 🐙Iris v3.8.0 Oct 5, 2023
@stephenworsley stephenworsley moved this from Candidate for next sprint to 📋 Backlog in 🐙Iris v3.8.0 Oct 5, 2023
@trexfeathers trexfeathers moved this from 🦎 Tamed to ⚔ In Development in 🐉 Dragon Taming Oct 11, 2023
@stephenworsley stephenworsley moved this from 📋 Backlog to 🔖Assigned in 🐙Iris v3.8.0 Oct 12, 2023
@stephenworsley stephenworsley moved this from 🔖Assigned to 🏗 In progress in 🐙Iris v3.8.0 Oct 13, 2023
@trexfeathers trexfeathers moved this from 🏗 In progress to 👀 In review in 🐙Iris v3.8.0 Nov 10, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to 🏁 Done in 🐙Iris v3.8.0 Nov 17, 2023
@github-project-automation github-project-automation bot moved this from ⚔ In Development to 💰 Finished in 🐉 Dragon Taming Nov 17, 2023
@scitools-ci scitools-ci bot removed this from 🚴 Peloton Dec 15, 2023
@scitools-ci scitools-ci bot removed this from 🚴 Peloton Dec 16, 2023
@bayliffe
Copy link
Contributor Author

@HGWright @trexfeathers @bjlittle thank you for your work on this. I'm afraid in the turmoil of operational releases, the progress made on this issue completely passed me by at the end of last year. Your efforts are much appreciated and will help us to avoid spurious behaviour in downstream systems ingesting probabilistic pressure data. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dragon 🐉 https://github.com/orgs/SciTools/projects/19?pane=info Feature: NetCDF + CF-conventions Type: Bug
Projects
Status: 💰 Finished
Status: 🏁 Done
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

8 participants