-
Notifications
You must be signed in to change notification settings - Fork 301
Speedup PP rules cube construction. #583
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
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,28 +200,37 @@ def __init__(self, coord, dims=None): | |
| dims = [dims] | ||
| self.dims = dims | ||
|
|
||
| def add_coord(self, cube): | ||
| def add_coord(self, cube, cube_coordnames_used, cube_dims_used): | ||
| added = False | ||
|
|
||
| # Check for unique names, so can use faster add_distinct_xx methods. | ||
| coord_names_entry = (self.coord.standard_name, | ||
| self.coord.long_name) | ||
| coord_names_unique = coord_names_entry not in cube_coordnames_used | ||
| if coord_names_unique: | ||
| cube_coordnames_used.append(coord_names_entry) | ||
|
|
||
| # Try to add to dim_coords? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. glad that this call to the cubes private method is gone |
||
| if isinstance(self.coord, iris.coords.DimCoord) and self.dims: | ||
| if len(self.dims) > 1: | ||
| raise Exception("Only 1 dim allowed for a DimCoord") | ||
| raise ValueError("Only 1 dim allowed for a DimCoord") | ||
|
|
||
| # Does the cube already have a coord for this dim? | ||
| already_taken = False | ||
| for coord, coord_dim in cube._dim_coords_and_dims: | ||
| if coord_dim == self.dims[0]: | ||
| already_taken = True | ||
| break | ||
|
|
||
| if not already_taken: | ||
| cube.add_dim_coord(self.coord, self.dims[0]) | ||
| coord_dim = self.dims[0] | ||
| if coord_dim not in cube_dims_used: | ||
| cube_dims_used.append(coord_dim) | ||
| if coord_names_unique: | ||
| cube._add_distinct_dim_coord(self.coord, coord_dim) | ||
| else: | ||
| cube.add_dim_coord(self.coord, coord_dim) | ||
| added = True | ||
|
|
||
| # If we didn't add it to dim_coords, add it to aux_coords. | ||
| # If we didn't add it to dim_coords, add it to aux_coords. | ||
| if not added: | ||
| cube.add_aux_coord(self.coord, self.dims) | ||
| if coord_names_unique: | ||
| cube._add_distinct_aux_coord(self.coord, self.dims) | ||
| else: | ||
| cube.add_aux_coord(self.coord, self.dims) | ||
|
|
||
| def __repr__(self): | ||
| return "<CoordAndDims: %r, %r>" % (self.coord.name, self.dims) | ||
|
|
@@ -351,7 +360,8 @@ def _create_action_method(self, i, action): | |
| pass | ||
|
|
||
| @abc.abstractmethod | ||
| def _process_action_result(self, obj, cube): | ||
| def _process_action_result(self, obj, cube, | ||
| cube_coordnames_used, cube_dims_used): | ||
| pass | ||
|
|
||
| def __repr__(self): | ||
|
|
@@ -381,7 +391,7 @@ def _matches_field(self, field): | |
| """Simple wrapper onto evaluates_true in the case where cube is None.""" | ||
| return self.evaluates_true(None, field) | ||
|
|
||
| def run_actions(self, cube, field): | ||
| def run_actions(self, cube, field, cube_coordnames_used, cube_dims_used): | ||
| """ | ||
| Adds to the given cube based on the return values of all the actions. | ||
|
|
||
|
|
@@ -410,7 +420,8 @@ def run_actions(self, cube, field): | |
| # Run this action. | ||
| obj = self._exec_actions[i](field, f, pp, grib, cm) | ||
| # Process the return value (if any), e.g a CM object or None. | ||
| action_factory = self._process_action_result(obj, cube) | ||
| action_factory = self._process_action_result( | ||
| obj, cube, cube_coordnames_used, cube_dims_used) | ||
| if action_factory: | ||
| factories.append(action_factory) | ||
|
|
||
|
|
@@ -435,15 +446,16 @@ def _create_action_method(self, i, action): | |
| # Add to our list of actions. | ||
| exec 'self._exec_actions.append(self._exec_action_%d)' % i | ||
|
|
||
| def _process_action_result(self, obj, cube): | ||
| def _process_action_result(self, obj, cube, | ||
| cube_coordnames_used, cube_dims_used): | ||
| """Process the result of an action.""" | ||
|
|
||
| factory = None | ||
|
|
||
| # NB. The names such as 'CoordAndDims' and 'CellMethod' are defined by | ||
| # the "deferred import" performed by Rule.run_actions() above. | ||
| if isinstance(obj, CoordAndDims): | ||
| obj.add_coord(cube) | ||
| obj.add_coord(cube, cube_coordnames_used, cube_dims_used) | ||
|
|
||
| #cell methods - not yet implemented | ||
| elif isinstance(obj, CellMethod): | ||
|
|
@@ -487,7 +499,7 @@ def _process_action_result(self, obj, cube): | |
|
|
||
| class ObjectReturningRule(FunctionRule): | ||
| """A rule which returns a list of objects when its actions are run.""" | ||
| def run_actions(self, cube, field): | ||
| def run_actions(self, cube, field, cube_coordnames_used, cube_dims_used): | ||
| f = pp = grib = field | ||
| cm = cube | ||
| return [action(field, f, pp, grib, cm) for action in self._exec_actions] | ||
|
|
@@ -503,7 +515,8 @@ def _create_action_method(self, i, action): | |
| # Add to our list of actions. | ||
| exec 'self._exec_actions.append(self._exec_action_%d)' % i | ||
|
|
||
| def _process_action_result(self, obj, cube): | ||
| def _process_action_result(self, obj, cube, | ||
| cube_coordnames_used, cube_dims_used): | ||
| # This should always be None, as our rules won't create anything. | ||
| pass | ||
|
|
||
|
|
@@ -628,10 +641,14 @@ def verify(self, cube, field): | |
| """ | ||
| matching_rules = [] | ||
| factories = [] | ||
| cube_coordnames_used = [] | ||
| cube_dims_used = [] | ||
| for rule in self._rules: | ||
| if rule.evaluates_true(cube, field): | ||
| matching_rules.append(rule) | ||
| rule_factories = rule.run_actions(cube, field) | ||
| rule_factories = rule.run_actions(cube, field, | ||
| cube_coordnames_used, | ||
| cube_dims_used) | ||
| if rule_factories: | ||
| factories.extend(rule_factories) | ||
| return RuleResult(cube, matching_rules, factories) | ||
|
|
@@ -805,8 +822,12 @@ def load_cubes(filenames, user_callback, loader): | |
|
|
||
| # Cross referencing | ||
| rules = loader.cross_ref_rules.matching_rules(field) | ||
| cube_coordnames_used = [] | ||
| cube_dims_used = [] | ||
| for rule in rules: | ||
| reference, = rule.run_actions(cube, field) | ||
| reference, = rule.run_actions(cube, field, | ||
| cube_coordnames_used, | ||
| cube_dims_used) | ||
| name = reference.name | ||
| # Register this cube as a source cube for the named | ||
| # reference. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks whether the coordinate is already 'used', however the more information checked against, the more often that it will correctly determine the answer, thus bypass the more hungry coordinate search (cube.coord). Why not also add 'var_name' here to catch more cases?