Repository: nismod/smif

Issue:
Adapter coefficient data should be keyed by the set of dimension names within a spec pair
Coefficients for performing conversions do not need to be generated for every Spec pair, but for every unique Spec dimension pairing.

Discussion:

Patch:
diff --git a/ci/install.sh b/ci/install.sh
index d6cac22b..2a4d8d29 100755
--- a/ci/install.sh
+++ b/ci/install.sh
@@ -15,20 +15,29 @@ if [[ "$DISTRIB" == "conda" ]]; then
     deactivate
 
     # Use the miniconda installer for faster download / install of conda
-    # itself
-    wget http://repo.continuum.io/miniconda/Miniconda-latest-Linux-x86_64.sh \
+    wget https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh \
         -O miniconda.sh
     chmod +x miniconda.sh && ./miniconda.sh -b -p $HOME/miniconda -f
     export PATH=$HOME/miniconda/bin:$PATH
     rm miniconda.sh -f
+    # Don't ask for confirmation
+    conda config --set always_yes true
+    # Update conda
+    conda update conda
+
+    # Follow channel priority strictly
+    conda config --set channel_priority strict
+    # Don't change prompt
+    conda config --set changeps1 false
+
+    if [[ "$PYTHON_VERSION" != "3.5" ]]; then
+        # Add conda-forge as priority channel
+        # conda-forge builds packages for Python 3.6 and above as of 2018-10-01
+        conda config --add channels conda-forge
+    fi
 
-    conda update --yes conda -c conda-forge
-
-    conda config --add channels conda-forge
-
-    # Configure the conda environment and put it in the path using the
-    # provided versions
-    conda create -n testenv --yes -c conda-forge \
+    # Create the conda environment
+    conda create -n testenv --yes \
         python=$PYTHON_VERSION \
         pytest \
         pytest-cov \
@@ -39,12 +48,9 @@ if [[ "$DISTRIB" == "conda" ]]; then
         pandas \
         psycopg2 \
         shapely \
-        fiona  && source activate testenv
+        fiona
 
-    if [[ "$PYTHON_VERSION" == "3.5" ]]; then
-        # Pin libgcc as possible root cause of fiona/shapely shared library import errors
-        conda install --yes libgcc-ng==7.2.0
-    fi
+    source activate testenv
 fi
 
 python setup.py develop
diff --git a/src/smif/convert/adaptor.py b/src/smif/convert/adaptor.py
index c08dd9e5..477188b8 100644
--- a/src/smif/convert/adaptor.py
+++ b/src/smif/convert/adaptor.py
@@ -48,14 +48,15 @@ class Adaptor(Model, metaclass=ABCMeta):
         -------
         numpy.ndarray
         """
+        from_dim, to_dim = self.get_convert_dims(from_spec, to_spec)
         try:
-            coefficients = data_handle.read_coefficients(from_spec, to_spec)
+            coefficients = data_handle.read_coefficients(from_dim, to_dim)
         except SmifDataNotFoundError:
             msg = "Generating coefficients for %s to %s"
-            self.logger.info(msg, from_spec, to_spec)
+            self.logger.info(msg, from_dim, to_dim)
 
             coefficients = self.generate_coefficients(from_spec, to_spec)
-            data_handle.write_coefficients(from_spec, to_spec, coefficients)
+            data_handle.write_coefficients(from_dim, to_dim, coefficients)
         return coefficients
 
     @abstractmethod
diff --git a/src/smif/data_layer/abstract_data_store.py b/src/smif/data_layer/abstract_data_store.py
index cd74508c..ee56c91d 100644
--- a/src/smif/data_layer/abstract_data_store.py
+++ b/src/smif/data_layer/abstract_data_store.py
@@ -180,17 +180,19 @@ class DataStore(metaclass=ABCMeta):
 
     # region Conversion coefficients
     @abstractmethod
-    def read_coefficients(self, source_spec, destination_spec):
+    def read_coefficients(self, source_dim, destination_dim):
         """Reads coefficients from the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `write_coefficients` implement caching of conversion
-        coefficients between dimensions.
+        coefficients between a single pair of dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim : str
+            dimension name
+        destination_dim : str
+            dimension name
 
         Returns
         -------
@@ -202,17 +204,19 @@ class DataStore(metaclass=ABCMeta):
         """
 
     @abstractmethod
-    def write_coefficients(self, source_spec, destination_spec, data):
+    def write_coefficients(self, source_dim, destination_dim, data):
         """Writes coefficients to the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `read_coefficients` implement caching of conversion
-        coefficients between dimensions.
+        coefficients between a single pair of dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim : str
+            dimension name
+        destination_dim : str
+            dimension name
         data : numpy.ndarray
 
         Notes
diff --git a/src/smif/data_layer/data_handle.py b/src/smif/data_layer/data_handle.py
index 90445772..62bd5855 100644
--- a/src/smif/data_layer/data_handle.py
+++ b/src/smif/data_layer/data_handle.py
@@ -562,44 +562,43 @@ class DataHandle(object):
             decision_iteration
         )
 
-    def read_coefficients(self,
-                          source_spec: Spec,
-                          destination_spec: Spec) -> np.ndarray:
+    def read_coefficients(self, source_dim: str, destination_dim: str) -> np.ndarray:
         """Reads coefficients from the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `write_coefficients` implement caching of conversion
         coefficients between dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim: str
+            Dimension name
+        destination_dim: str
+            Dimension name
 
         Returns
         -------
         numpy.ndarray
         """
-        data = self._store.read_coefficients(source_spec, destination_spec)
+        data = self._store.read_coefficients(source_dim, destination_dim)
         return data
 
-    def write_coefficients(self,
-                           source_spec: Spec,
-                           destination_spec: Spec,
-                           data: np.ndarray):
+    def write_coefficients(self, source_dim: str, destination_dim: str, data: np.ndarray):
         """Writes coefficients to the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `read_coefficients` implement caching of conversion
         coefficients between dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim: str
+            Dimension name
+        destination_dim: str
+            Dimension name
         data : numpy.ndarray
         """
-        data = self._store.write_coefficients(source_spec, destination_spec, data)
+        data = self._store.write_coefficients(source_dim, destination_dim, data)
         return data
 
 
diff --git a/src/smif/data_layer/database_interface.py b/src/smif/data_layer/database_interface.py
index fa25703d..299b949c 100644
--- a/src/smif/data_layer/database_interface.py
+++ b/src/smif/data_layer/database_interface.py
@@ -1271,10 +1271,10 @@ class DbDataStore(DataStore):
     # endregion
 
     # region Conversion coefficients
-    def read_coefficients(self, source_spec, destination_spec):
+    def read_coefficients(self, source_dim, destination_dim):
         raise NotImplementedError
 
-    def write_coefficients(self, source_spec, destination_spec, data):
+    def write_coefficients(self, source_dim, destination_dim, data):
         raise NotImplementedError()
     # endregion
 
diff --git a/src/smif/data_layer/file/file_data_store.py b/src/smif/data_layer/file/file_data_store.py
index 3d26dbfa..326848d0 100644
--- a/src/smif/data_layer/file/file_data_store.py
+++ b/src/smif/data_layer/file/file_data_store.py
@@ -203,27 +203,26 @@ class FileDataStore(DataStore):
     # endregion
 
     # region Conversion coefficients
-    def read_coefficients(self, source_spec, destination_spec):
-        results_path = self._get_coefficients_path(source_spec, destination_spec)
+    def read_coefficients(self, source_dim, destination_dim):
+        results_path = self._get_coefficients_path(source_dim, destination_dim)
         try:
             return self._read_ndarray(results_path)
         except FileNotFoundError:
             msg = "Could not find the coefficients file for %s to %s"
-            self.logger.warning(msg, source_spec, destination_spec)
-            raise SmifDataNotFoundError(msg.format(source_spec, destination_spec))
+            self.logger.warning(msg, source_dim, destination_dim)
+            raise SmifDataNotFoundError(msg.format(source_dim, destination_dim))
 
-    def write_coefficients(self, source_spec, destination_spec, data):
-        results_path = self._get_coefficients_path(source_spec, destination_spec)
-        header = "Conversion coefficients {}:{}".format(
-            source_spec.name, destination_spec.name)
+    def write_coefficients(self, source_dim, destination_dim, data):
+        results_path = self._get_coefficients_path(source_dim, destination_dim)
+        header = "Conversion coefficients {}:{}".format(source_dim, destination_dim)
         self._write_ndarray(results_path, data, header)
 
-    def _get_coefficients_path(self, source_spec, destination_spec):
+    def _get_coefficients_path(self, source_dim, destination_dim):
         path = os.path.join(
             self.data_folders['coefficients'],
-            "{}_{}.{}_{}.{}".format(
-                source_spec.name, "-".join(source_spec.dims),
-                destination_spec.name, "-".join(destination_spec.dims),
+            "{}.{}.{}".format(
+                source_dim,
+                destination_dim,
                 self.coef_ext
             )
         )
diff --git a/src/smif/data_layer/memory_interface.py b/src/smif/data_layer/memory_interface.py
index ff7616c9..d64de539 100644
--- a/src/smif/data_layer/memory_interface.py
+++ b/src/smif/data_layer/memory_interface.py
@@ -362,16 +362,15 @@ class MemoryDataStore(DataStore):
     # endregion
 
     # region Conversion coefficients
-    def read_coefficients(self, source_spec, destination_spec):
-        spec = (source_spec.name, destination_spec.name)
+    def read_coefficients(self, source_dim, destination_dim):
         try:
-            return self._coefficients[(source_spec.name, destination_spec.name)]
+            return self._coefficients[(source_dim, destination_dim)]
         except KeyError:
-            msg = "Could not find coefficients for spec pair {}.{}"
-            raise SmifDataNotFoundError(msg.format(spec[0], spec[1]))
+            msg = "Could not find coefficients for conversion from {}>{}"
+            raise SmifDataNotFoundError(msg.format(source_dim, destination_dim))
 
-    def write_coefficients(self, source_spec, destination_spec, data):
-        self._coefficients[(source_spec.name, destination_spec.name)] = data
+    def write_coefficients(self, source_dim, destination_dim, data):
+        self._coefficients[(source_dim, destination_dim)] = data
     # endregion
 
     # region Results
diff --git a/src/smif/data_layer/store.py b/src/smif/data_layer/store.py
index dd645ad8..e2756a73 100644
--- a/src/smif/data_layer/store.py
+++ b/src/smif/data_layer/store.py
@@ -740,19 +740,19 @@ class Store():
     # endregion
 
     # region Conversion coefficients
-    def read_coefficients(self,
-                          source_spec: Spec,
-                          destination_spec: Spec) -> np.ndarray:
+    def read_coefficients(self, source_dim: str, destination_dim: str) -> np.ndarray:
         """Reads coefficients from the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `write_coefficients` implement caching of conversion
         coefficients between dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim : str
+            Dimension name
+        destination_dim : str
+            Dimension name
 
         Returns
         -------
@@ -762,29 +762,28 @@ class Store():
         -----
         To be called from :class:`~smif.convert.adaptor.Adaptor` implementations.
         """
-        return self.data_store.read_coefficients(source_spec, destination_spec)
+        return self.data_store.read_coefficients(source_dim, destination_dim)
 
-    def write_coefficients(self,
-                           source_spec: Spec,
-                           destination_spec: Spec,
-                           data: np.ndarray):
+    def write_coefficients(self, source_dim: str, destination_dim: str, data: np.ndarray):
         """Writes coefficients to the store
 
-        Coefficients are uniquely identified by their source/destination specs.
+        Coefficients are uniquely identified by their source/destination dimensions.
         This method and `read_coefficients` implement caching of conversion
         coefficients between dimensions.
 
         Parameters
         ----------
-        source_spec : ~smif.metadata.spec.Spec
-        destination_spec : ~smif.metadata.spec.Spec
+        source_dim : str
+            Dimension name
+        destination_dim : str
+            Dimension name
         data : numpy.ndarray
 
         Notes
         -----
         To be called from :class:`~smif.convert.adaptor.Adaptor` implementations.
         """
-        self.data_store.write_coefficients(source_spec, destination_spec, data)
+        self.data_store.write_coefficients(source_dim, destination_dim, data)
     # endregion
 
     # region Results


Test patch:
diff --git a/tests/data_layer/test_data_handle.py b/tests/data_layer/test_data_handle.py
index b3d13839..7061691b 100644
--- a/tests/data_layer/test_data_handle.py
+++ b/tests/data_layer/test_data_handle.py
@@ -802,42 +802,32 @@ class TestDataHandleCoefficients:
     """Tests the interface for reading and writing coefficients
     """
     def test_read_coefficient_raises(self, mock_store, mock_model):
-        """
+        """Reading coefficients that aren't cached should raise SmifDataNotFoundError
         """
         store = mock_store
-        source_spec = mock_model.outputs['gas_demand']
-        sink_spec = mock_model.outputs['gas_demand']
-
         dh = DataHandle(store, 1, 2010, [2010], mock_model)
+
         with raises(SmifDataNotFoundError):
-            dh.read_coefficients(source_spec, sink_spec)
+            dh.read_coefficients('from_dim', 'to_dim')
 
     def test_write_coefficients(self, mock_store, mock_model):
+        """Writing coefficients should succeed
         """
-        """
-
         store = mock_store
-        source_spec = mock_model.outputs['gas_demand']
-        sink_spec = mock_model.outputs['gas_demand']
         data = np.zeros(4)
 
         dh = DataHandle(store, 1, 2010, [2010], mock_model)
-        dh.write_coefficients(source_spec, sink_spec, data)
+        dh.write_coefficients('from_dim', 'to_dim', data)
 
     def test_read_coefficient_(self, mock_store, mock_model):
-        """
+        """Reading written coefficients should return the cached data
         """
         store = mock_store
-        source_spec = mock_model.outputs['gas_demand']
-        sink_spec = mock_model.outputs['gas_demand']
-
         dh = DataHandle(store, 1, 2010, [2010], mock_model)
-
         data = np.zeros(4)
-        dh.write_coefficients(source_spec, sink_spec, data)
-
-        actual = dh.read_coefficients(source_spec, sink_spec)
+        dh.write_coefficients('from_dim', 'to_dim', data)
 
+        actual = dh.read_coefficients('from_dim', 'to_dim')
         np.testing.assert_equal(actual, np.array([0, 0, 0, 0]))
 
 
diff --git a/tests/data_layer/test_data_store.py b/tests/data_layer/test_data_store.py
index 5ce339fe..a4cb981d 100644
--- a/tests/data_layer/test_data_store.py
+++ b/tests/data_layer/test_data_store.py
@@ -144,16 +144,15 @@ class TestState():
 class TestCoefficients():
     """Read/write conversion coefficients
     """
-    def test_read_write_coefficients(self, conversion_source_spec, conversion_sink_spec,
-                                     handler):
+    def test_read_write_coefficients(self, handler):
 
         with raises(SmifDataNotFoundError):
-            handler.read_coefficients(conversion_source_spec, conversion_sink_spec)
+            handler.read_coefficients('from_dim_name', 'to_dim_name')
 
         expected = np.array([[2]])
-        handler.write_coefficients(conversion_source_spec, conversion_sink_spec, expected)
+        handler.write_coefficients('from_dim_name', 'to_dim_name', expected)
 
-        actual = handler.read_coefficients(conversion_source_spec, conversion_sink_spec)
+        actual = handler.read_coefficients('from_dim_name', 'to_dim_name')
         np.testing.assert_equal(actual, expected)
 
 
diff --git a/tests/data_layer/test_data_store_csv.py b/tests/data_layer/test_data_store_csv.py
index 315b80e4..e74cf4b1 100644
--- a/tests/data_layer/test_data_store_csv.py
+++ b/tests/data_layer/test_data_store_csv.py
@@ -340,7 +340,8 @@ class TestNarrativeVariantData:
             config_handler.read_model_parameter_default('default.csv', spec)
 
         msg = "Data for 'test' contains duplicate values at [{'a': 2, 'b': 4}]"
-        assert msg in str(ex)
+        msg_alt = "Data for 'test' contains duplicate values at [{'b': 4, 'a': 2}]"
+        assert msg in str(ex) or msg_alt in str(ex)
 
     def test_error_wrong_name(self, config_handler):
         spec = Spec(
@@ -626,26 +627,17 @@ class TestWarmStart:
 class TestCoefficients:
     """Dimension conversion coefficients should be cached to disk and read if available.
     """
-    @fixture
-    def from_spec(self):
-        return Spec(name='from_test_coef', dtype='int')
-
-    @fixture
-    def to_spec(self):
-        return Spec(name='to_test_coef', dtype='int')
-
-    def test_read_write(self, from_spec, to_spec, config_handler):
+    def test_read_write(self, config_handler):
         data = np.eye(1000)
         handler = config_handler
-        handler.write_coefficients(from_spec, to_spec, data)
-        actual = handler.read_coefficients(from_spec, to_spec)
+        handler.write_coefficients('from_dim_name', 'to_dim_name', data)
+        actual = handler.read_coefficients('from_dim_name', 'to_dim_name')
         np.testing.assert_equal(actual, data)
 
-    def test_read_raises(self, from_spec, to_spec, config_handler):
+    def test_read_raises(self, config_handler):
         handler = config_handler
-        missing_spec = Spec(name='missing_coef', dtype='int')
         with raises(SmifDataNotFoundError):
-            handler.read_coefficients(missing_spec, to_spec)
+            handler.read_coefficients('wrong_dim_name', 'to_dim_name')
 
     def test_dfi_raises_if_folder_missing(self):
         """Ensure we can write files, even if project directory starts empty
diff --git a/tests/data_layer/test_store.py b/tests/data_layer/test_store.py
index ed9e982b..e4faa253 100644
--- a/tests/data_layer/test_store.py
+++ b/tests/data_layer/test_store.py
@@ -289,14 +289,13 @@ class TestStoreData():
         # read
         assert store.read_state('model_run_name', 0, 0) == state
 
-    def test_conversion_coefficients(self, store, conversion_coefficients,
-                                     conversion_source_spec, conversion_sink_spec):
+    def test_conversion_coefficients(self, store, conversion_coefficients):
         # write
         store.write_coefficients(
-            conversion_source_spec, conversion_sink_spec, conversion_coefficients)
+            'source_dim', 'sink_dim', conversion_coefficients)
         # read
         numpy.testing.assert_equal(
-            store.read_coefficients(conversion_source_spec, conversion_sink_spec),
+            store.read_coefficients('source_dim', 'sink_dim'),
             conversion_coefficients
         )