Skip to content

Fast add coords#637

Merged
bblay merged 2 commits intoSciTools:masterfrom
rhattersley:fast-add-coords
Aug 1, 2013
Merged

Fast add coords#637
bblay merged 2 commits intoSciTools:masterfrom
rhattersley:fast-add-coords

Conversation

@rhattersley
Copy link
Member

This PR has two main aspects:

  1. It modifies the PP/GRIB load rules (via SciTools/iris-code-generators#8 which should be merged first) so all the coordinates are collated instead of added directly to the Cube. (Similar changes are also applied to non-coordinate metadata.) The coordinates are then supplied to the Cube constructor.
  2. It speeds up the addition of coordinates during the Cube constructor by using a quick uniqueness check where possible instead of the full, slow self.coords(coord=coord) check done by Cube.add_dim_coords() and Cube.add_aux_coords().

Approx 7% performance boost doing iris.load_cubes() on a 17MB PP file containing 2048 fields.

@rhattersley
Copy link
Member Author

Since this branch now has merge conflicts and no one has commented yet I'll rebase ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider moving this logic into add_dim_coords and add_aux_coords so other code can take advantage of it?

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, actually it's not so applicable outside here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This logic only works when adding a bunch of coordinates in a single operation, so it isn't suitable for either of those methods. It's not possible to have a simple, persistent cache of identities simply because everything about a coordinate's identity is mutable!

Copy link
Contributor

Choose a reason for hiding this comment

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

yep

@rhattersley
Copy link
Member Author

If there's nothing else, are you happy for me to rebase now?

@rhattersley
Copy link
Member Author

Rebased from 418156c6664085a5344c85049f849b9159669bc2 to 336e4a1.

bblay added a commit that referenced this pull request Aug 1, 2013
@bblay bblay merged commit 901525a into SciTools:master Aug 1, 2013
@rhattersley rhattersley deleted the fast-add-coords branch August 1, 2013 13:17
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.

2 participants