From 4863d153624d7c84a7982428d3f9ff36e7910b4f Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 24 Jun 2013 15:36:46 +0100 Subject: [PATCH 1/2] Speedup rules add_coord, by using lower-level versions of cube.add_xxx_coord. --- lib/iris/cube.py | 23 +++++++++++-- lib/iris/fileformats/rules.py | 63 +++++++++++++++++++++++------------ lib/iris/tests/test_rules.py | 3 +- 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 63add04481..6b97adf8dd 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -602,6 +602,18 @@ def add_aux_coord(self, coord, data_dims=None): See also :meth:`Cube.remove_coord()`. + """ + if self.coords(coord=coord): + raise ValueError('Duplicate coordinates are not permitted.') + self._add_distinct_aux_coord(coord, data_dims) + + def _add_distinct_aux_coord(self, coord, data_dims=None): + """ + Add an AuxCoord already known to be different from all existing ones. + + This is just a speedup version of add_aux_coord, to avoid comparing + the new coord against all the existing ones. + """ # Convert to a tuple of integers @@ -629,8 +641,6 @@ def add_aux_coord(self, coord, data_dims=None): raise ValueError('You must supply the data-dimensions for ' 'multi-valued coordinates.') - if self.coords(coord=coord): # TODO: just fail on duplicate object - raise ValueError('Duplicate coordinates are not permitted.') self._aux_coords_and_dims.append([coord, data_dims]) def add_aux_factory(self, aux_factory): @@ -669,6 +679,15 @@ def add_dim_coord(self, dim_coord, data_dim): if self.coords(coord=dim_coord): raise ValueError('The coordinate already exists on the cube. ' 'Duplicate coordinates are not permitted.') + self._add_distinct_dim_coord(dim_coord, data_dim) + + def _add_distinct_dim_coord(self, dim_coord, data_dim): + """ + Add a DimCoord already known to be different from all existing ones. + + This is just a speedup version of add_dim_coord, to avoid comparing + the new coord against all the existing ones. + """ if isinstance(data_dim, collections.Container) and len(data_dim) != 1: raise ValueError('The supplied data dimension must be a single ' 'number.') diff --git a/lib/iris/fileformats/rules.py b/lib/iris/fileformats/rules.py index e9d18c7a10..e52c7fab55 100644 --- a/lib/iris/fileformats/rules.py +++ b/lib/iris/fileformats/rules.py @@ -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? if isinstance(self.coord, iris.coords.DimCoord) and self.dims: if len(self.dims) > 1: raise Exception("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 "" % (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,7 +446,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): """Process the result of an action.""" factory = None @@ -443,7 +455,7 @@ def _process_action_result(self, obj, cube): # 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. diff --git a/lib/iris/tests/test_rules.py b/lib/iris/tests/test_rules.py index f5e40f2afc..e274366ce9 100644 --- a/lib/iris/tests/test_rules.py +++ b/lib/iris/tests/test_rules.py @@ -162,7 +162,8 @@ def test_cross_reference(self): # A fake cross-reference rule set ref = ReferenceTarget('orography', None) orog_xref_rule = Mock() - orog_xref_rule.run_actions = lambda cube, field: (ref,) + orog_xref_rule.run_actions = \ + lambda cube, field, _used_coords, _used_dims: (ref,) xref_rules = Mock() xref_rules.matching_rules = lambda field: \ [orog_xref_rule] if field is orog_field else [] From 8d8c3a821ea603366644e4c38f0847a779289098 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 1 Jul 2013 14:52:08 +0100 Subject: [PATCH 2/2] Use more suitable Exception class. --- lib/iris/fileformats/rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/fileformats/rules.py b/lib/iris/fileformats/rules.py index e52c7fab55..3f9a87bf31 100644 --- a/lib/iris/fileformats/rules.py +++ b/lib/iris/fileformats/rules.py @@ -213,7 +213,7 @@ def add_coord(self, cube, cube_coordnames_used, cube_dims_used): # Try to add to dim_coords? 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? coord_dim = self.dims[0]