Skip to content

Distinguish between out_name and var_name#391

Closed
jvegreg wants to merge 19 commits into
masterfrom
dev_use_out_name
Closed

Distinguish between out_name and var_name#391
jvegreg wants to merge 19 commits into
masterfrom
dev_use_out_name

Conversation

@jvegreg

@jvegreg jvegreg commented Dec 9, 2019

Copy link
Copy Markdown
Contributor

@sloosvel (#333) found that some tables contain variables with the 'out_name' different from the 'var_name'. We are currently assuming that both are the same, this pull request fixes this.

Closes #333

@jvegreg jvegreg added bug Something isn't working cmor Related to the CMOR standard labels Dec 9, 2019
@sloosvel sloosvel marked this pull request as ready for review December 9, 2019 13:32
@sloosvel

sloosvel commented Dec 9, 2019

Copy link
Copy Markdown
Contributor

Thanks @jvegasbsc , it looks like it works well. Although, the variable needs to be called in the recipe by the var_name, not the out_name. Maybe this could become confusing if someone is trying to work with 6hourly files named zg_... .nc , because in the recipe they would need to call zg7h (or zg27) to be able to load the files properly.

@valeriupredoi

Copy link
Copy Markdown
Contributor

yeah am with Saskia on this one

@jvegreg

jvegreg commented Dec 12, 2019

Copy link
Copy Markdown
Contributor Author

Yes, that is inconvenient. But i have no idea on how I can distinguish between them, as they belong to the same table

@bouweandela bouweandela changed the base branch from development to master January 3, 2020 12:37

@bouweandela bouweandela left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the choice var_name is not very clear, because this is also the name of the iris cube attribute that contains the short_name. Would it be possible to use something else? Maybe cmor_name instead of var_name?

Note that this pull request breaks existing recipes in which short_name is defined, i.e.:
examples/recipe_variable_groups.yml
examples/recipe_check_obs.yml
recipe_flato13ipcc.yml
recipe_tcr.yml
recipe_ecs.yml
recipe_anav13jclim.yml

@jvegreg

jvegreg commented Jan 9, 2020

Copy link
Copy Markdown
Contributor Author

I think the choice var_name is not very clear, because this is also the name of the iris cube attribute that contains the short_name. Would it be possible to use something else? Maybe cmor_name instead of var_name?

Good idea. I will change it

Note that this pull request breaks existing recipes in which short_name is defined, i.e.:
examples/recipe_variable_groups.yml
examples/recipe_check_obs.yml
recipe_flato13ipcc.yml
recipe_tcr.yml
recipe_ecs.yml
recipe_anav13jclim.yml

Ok. I will create a companion branch in ESMValTool to fix them

Comment thread esmvalcore/cmor/fix.py Outdated
@bouweandela

Copy link
Copy Markdown
Member

Can you also check what name is used in the drs? The short name or the cmor name?

@jvegreg

jvegreg commented Jan 10, 2020

Copy link
Copy Markdown
Contributor Author

It is the short_name. It is not a problem because the variablse that share table and out_name are never present together in the data request for any activity. We need to distinguish them to be able to check against the correct definition, though

@bouweandela

Copy link
Copy Markdown
Member

@jvegasbsc Could you have a look at #391 (comment)? Apart from that, I think this pull request looks fine.

@bouweandela

Copy link
Copy Markdown
Member

@jvegasbsc Could you a have a look at the merge conflicts and failing tests?

bouweandela
bouweandela previously approved these changes Feb 26, 2020

@bouweandela bouweandela left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me now. @mattiarighi Could you please test, using the branch from ESMValGroup/ESMValTool#1493?

@mattiarighi

Copy link
Copy Markdown
Contributor

I tested successfully a few recipes and cmorizers, other needs still to be adjusted (I reported here).

@bouweandela bouweandela dismissed their stale review March 16, 2020 10:43

Outdated

@jvegreg jvegreg added this to the v2.0.0 milestone Jun 8, 2020
@bouweandela

Copy link
Copy Markdown
Member

@jvegasbsc Instead of the massive renaming approach, I would be more in favour of solving this with minimal changes, similar to how it was done for the variables with different names between CMIP5 and CMIP6 in #595. Do you think that would be possible?

@jvegreg

jvegreg commented Oct 21, 2020

Copy link
Copy Markdown
Contributor Author

@jvegasbsc Instead of the massive renaming approach, I would be more in favour of solving this with minimal changes, similar to how it was done for the variables with different names between CMIP5 and CMIP6 in #595. Do you think that would be possible?

Not, because it is not a massive rename but a splitting one name in two. Currently we are treating two different names as if they are only one.

I kept short_name for the out_name, as it is the most widely used (specially in diagnostics) and only use the new cmor_name when working with the tables, which is mostly internal stuff.

@bouweandela

Copy link
Copy Markdown
Member

It will lead to a massive rename in the ESMValTool repository as it is now (partly implemented in ESMValGroup/ESMValTool#1493), because we will need to adjust the recipes/diagnostics/cmorizers to the fact that one name is split into two.

Would it lead to less renaming if we keep short_name for the variable name in the table and call out_name var_name so it aligns with iris?

@jvegreg

jvegreg commented Oct 26, 2020

Copy link
Copy Markdown
Contributor Author

Would it lead to less renaming if we keep short_name for the variable name in the table and call out_name var_name so it aligns with iris?

In the recipes, yes. In ESMValCore code I think we will have more, but involve less people...

@bouweandela bouweandela removed this from the v2.1.0 milestone Dec 9, 2020
@sloosvel sloosvel added this to the v2.3.0 milestone May 4, 2021
@bouweandela

Copy link
Copy Markdown
Member

Shall we plan a short meeting at the workshop to discuss how to go forward with this @jvegasbsc @sloosvel?

@sloosvel

sloosvel commented May 4, 2021

Copy link
Copy Markdown
Contributor

Fine for me!

@bouweandela

bouweandela commented May 5, 2021

Copy link
Copy Markdown
Member

Summary of the discussion: instead of the approach here, an extra name, e.g. cmor_name will become available in the variable definition section of the recipe to specify the name of the variable in the cmor table if it differs from short_name. This name will default to short_name if it is not provided. This ensures backward compatiblity.

@jvegreg

jvegreg commented May 5, 2021

Copy link
Copy Markdown
Contributor Author

Replaced by #1099

@jvegreg jvegreg closed this May 5, 2021
@bouweandela bouweandela deleted the dev_use_out_name branch March 13, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cmor Related to the CMOR standard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem loading zg for 6hrPlevPt

6 participants