Skip to content

Remove redundant and erroneous standard_name to LBFC/STASH PP save rules.#190

Merged
rhattersley merged 2 commits intoSciTools:masterfrom
bjlittle:redundant_pp_save_rules
Nov 7, 2012
Merged

Remove redundant and erroneous standard_name to LBFC/STASH PP save rules.#190
rhattersley merged 2 commits intoSciTools:masterfrom
bjlittle:redundant_pp_save_rules

Conversation

@bjlittle
Copy link
Member

@bjlittle bjlittle commented Nov 7, 2012

Working with AQUM revealed that loading a Fieldsfile with a PP-field of stash code 2 is translated to a cube with standard_name of eastward_wind, and that a PP-field of stash code 3 is translated to a cube with standard_name of northward_wind.

On saving such cubes, the PP save rules set/clobber the LBFC and LBUSER[3] (stash section and item) based only on the standard_name of the cube:

  • For eastward_wind it forces 56 and 15201,
  • For northward_wind it forces 57 and 15202.

These PP save rules are clearly wrong. It is not safe to translate from standard_name to stash code.

These save rules have been removed, along with several other such similar rules. All test still pass after removal, so thankfully we don't have current tests that rely on this erroneous behaviour.

@rhattersley
Copy link
Member

I've spoken with @marqh and we've agreed that it be nice to keep the LBFC mappings. Would that still solve your problem?

@bjlittle
Copy link
Member Author

bjlittle commented Nov 7, 2012

Okay, that works for me.

I've checked the source AQUM FF and all PP-fields with a stash code of 2 have an LBFC = 56, and all PP-fields with a stash code of 3 have an LBFC = 57.

Excellent spot @rhattersley! Otherwise I would have lost the LBFC on saving. I'll resubmit the PR 😃

@rhattersley
Copy link
Member

Do the tests, extests, doctests, etc. all pass already? (Oh, for a working Travis-CI config ;-))

@bjlittle
Copy link
Member Author

bjlittle commented Nov 7, 2012

Yes, yes and yes.

Although, there are (I assume) pre-existing failures on master for unittests and extests.

A fresh branch cut from upstream/master has a FAIL: test_simple (iris.tests.test_mapping.TestMappingSubRegion), and the extest failure FAIL: test_lagged_ensemble (example_tests.test_lagged_ensemble.TestLaggedEnsemble).

These are both image differences.

@rhattersley
Copy link
Member

These are both image differences.

You have to be careful which version of Cartopy you run the tests against. And when I say "careful", I probably mean "lucky" as it's not exactly clear which version you should be using. (Oh, for a working Travis setup which would formally capture some of this! ;-))

I've run the tests myself and it's lookin' good, so I'm happy to merge.

rhattersley added a commit that referenced this pull request Nov 7, 2012
Remove erroneous standard_name to STASH PP save rules.
@rhattersley rhattersley merged commit f41e80e into SciTools:master Nov 7, 2012
@pelson
Copy link
Member

pelson commented Nov 8, 2012

Cartopy v0.4.0?

@cpelley cpelley mentioned this pull request Jan 24, 2014
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants