Minor clean-up to gmxapi package.
authorM. Eric Irrgang <ericirrgang@gmail.com>
Thu, 26 Sep 2019 12:17:49 +0000 (15:17 +0300)
committerEric Irrgang <ericirrgang@gmail.com>
Wed, 16 Oct 2019 15:07:06 +0000 (17:07 +0200)
* Resolve some linting warnings and potential import ambiguities.
* Apply recommended pybind iteration mechanism for Python objects.
* Remove some extraneous syntax and imports.
* Make explicit a call that relied on positional argument processing.
* Alias an imported name (Future) to avoid namespace collision.
* Normalize initialization of DataSourceCollection.
  * Make sure that initialization at construction uses the same setter
    code path as subscripted assignment on the object instances.
  * Add ResultDescription to gmxapi.operation.Future.

Change-Id: Ia687929302edd85a0af616b1d947db21e2f3876e

python_packaging/src/gmxapi/export_context.cpp
python_packaging/src/gmxapi/export_system.cpp
python_packaging/src/gmxapi/operation.py
python_packaging/src/gmxapi/pycontext.cpp
python_packaging/src/gmxapi/simulation/context.py
python_packaging/src/gmxapi/simulation/fileio.py
python_packaging/src/gmxapi/simulation/modify_input.py
python_packaging/src/gmxapi/typing.py

index 93cd76c509a4177cc5656c7c8d5891ee24785908..3bea021d40707d777fce3c72483d2d817eb868b7 100644 (file)
@@ -81,9 +81,11 @@ static void setMDArgs(std::vector<std::string>* mdargs,
             throw gmxapi::UsageError("Grid argument must describe domain decomposition grid.");
         }
         std::vector<std::string> vals;
-        for (auto && val : params["grid"])
+        auto                     iterator = py::iter(params["grid"]);
+        while (iterator != py::iterator::sentinel())
         {
-            vals.emplace_back(py::cast<std::string>(py::str(val)));
+            vals.emplace_back(py::cast<std::string>(py::str(iterator)));
+            ++iterator;
         }
         mdargs->emplace_back("-dd");
         for (auto && val : vals)
index 752e6d1f50ea76915ce1286686b260f4f4b69483..ea91629fbc38f188b7c8b7b72551479247037362 100644 (file)
@@ -41,9 +41,6 @@
 
 #include "module.h"
 
-#include <iostream>
-
-#include "gmxapi/md.h"
 #include "gmxapi/session.h"
 #include "gmxapi/status.h"
 #include "gmxapi/system.h"
index 7c31bd1654f604e314f355cddca9ce5d27561825..f4f41974e6fd2b96772b83b7c0352c82d50e1c46 100644 (file)
@@ -67,6 +67,7 @@ from gmxapi import exceptions
 from gmxapi import logger as root_logger
 from gmxapi.abc import OperationImplementation, MutableResource, Node
 from gmxapi.typing import _Context, ResultTypeVar, SourceTypeVar, valid_result_types, valid_source_types
+from gmxapi.typing import Future as _Future
 
 # Initialize module-level logger
 logger = root_logger.getChild('operation')
@@ -197,43 +198,27 @@ class DataSourceCollection(collections.OrderedDict):
 
         Data sources may be any of the basic gmxapi data types, gmxapi Futures
         of those types, or gmxapi ensemble data bundles of the above.
-
-        Note that the checking and conditioning could be moved to one or more
-        creation functions. In conjunction with an InputCollectionDescription,
-        such creation functions could make decisions about automatically shaping
-        the data flow topology or making conversions of data type or shape.
         """
-        named_data = []
+        super(DataSourceCollection, self).__init__()
         for name, value in kwargs.items():
-            if not isinstance(name, str):
-                raise exceptions.TypeError('Data must be named with str type.')
-            # TODO: Encapsulate handling of proferred data sources to Context details.
-            # Preprocessed input should be self-describing gmxapi data types. Structured
-            # input must be recursively (depth-first) converted to gmxapi data types.
-            # TODO: Handle gmxapi Futures stored as dictionary elements!
-            if not isinstance(value, valid_source_types):
-                if isinstance(value, collections.abc.Iterable):
-                    # Iterables here are treated as arrays, but we do not have a robust typing system.
-                    # Warning: In the initial implementation, the iterable may contain Future objects.
-                    # TODO: (#2993) Revisit as we sort out data shape and Future protocol.
-                    value = datamodel.ndarray(value)
-                elif hasattr(value, 'result'):
-                    # A Future object.
-                    pass
-                else:
-                    raise exceptions.ApiError('Cannot process data source {}'.format(value))
-            named_data.append((name, value))
-        super().__init__(named_data)
+            self[name] = value
 
     def __setitem__(self, key: str, value: SourceTypeVar) -> None:
         if not isinstance(key, str):
             raise exceptions.TypeError('Data must be named with str type.')
+        # TODO: Encapsulate handling of proferred data sources to Context details.
+        # Preprocessed input should be self-describing gmxapi data types. Structured
+        # input must be recursively (depth-first) converted to gmxapi data types.
+        # TODO: Handle gmxapi Futures stored as dictionary elements!
         if not isinstance(value, valid_source_types):
             if isinstance(value, collections.abc.Iterable):
                 # Iterables here are treated as arrays, but we do not have a robust typing system.
                 # Warning: In the initial implementation, the iterable may contain Future objects.
                 # TODO: (#2993) Revisit as we sort out data shape and Future protocol.
-                value = datamodel.ndarray(value)
+                try:
+                    value = datamodel.ndarray(value)
+                except (exceptions.ValueError, exceptions.TypeError) as e:
+                    raise exceptions.TypeError('Iterable could not be converted to NDArray: {}'.format(value)) from e
             elif hasattr(value, 'result'):
                 # A Future object.
                 pass
@@ -1328,7 +1313,7 @@ class OperationDetailsBase(OperationImplementation, InputDescription,
 #         """Recreate the Operation at the consuming end of the DataEdge."""
 
 
-class Future(typing.Generic[ResultTypeVar]):
+class Future(_Future):
     """gmxapi data handle.
 
     Future is currently more than a Future right now. (should be corrected / clarified.)
@@ -1343,14 +1328,18 @@ class Future(typing.Generic[ResultTypeVar]):
 
     TODO: extract the abstract class for input inspection?
     Currently abstraction is handled through SourceResource subclassing.
+
+    Attributes:
+        description (ResultDescription): Describes the result to be obtained from this Future.
+
     """
 
     def __init__(self, resource_manager: SourceResource, name: str, description: ResultDescription):
         self.name = name
         if not isinstance(description, ResultDescription):
             raise exceptions.ValueError('Need description of requested data.')
+        self.description = description  # type: ResultDescription
         self.resource_manager = resource_manager
-        self.description = description
 
         # Deprecated. We should not "reset" futures, but reconstitute them, but we
         # need to move the data model to a subscription-based system so that we can
@@ -2613,7 +2602,7 @@ def _make_datastore(output_description: OutputCollectionDescription, ensemble_wi
     datastore = collections.OrderedDict()
     for name, dtype in output_description.items():
         assert isinstance(dtype, type)
-        result_description = ResultDescription(dtype, width=ensemble_width)
+        result_description = ResultDescription(dtype=dtype, width=ensemble_width)
         datastore[name] = OutputData(name=name, description=result_description)
     return datastore
 
@@ -3442,7 +3431,7 @@ def join_arrays(*, front: datamodel.NDArray = (), back: datamodel.NDArray = ())
 Scalar = typing.TypeVar('Scalar')
 
 
-def concatenate_lists(sublists: list = ()) -> Future[gmx.datamodel.NDArray]:
+def concatenate_lists(sublists: list = ()) -> _Future[gmx.datamodel.NDArray]:
     """Combine data sources into a single list.
 
     A trivial data flow restructuring operation.
@@ -3458,7 +3447,7 @@ def concatenate_lists(sublists: list = ()) -> Future[gmx.datamodel.NDArray]:
                                             concatenate_lists(sublists[1:])))
 
 
-def make_constant(value: Scalar) -> Future:
+def make_constant(value: Scalar) -> _Future:
     """Provide a predetermined value at run time.
 
     This is a trivial operation that provides a (typed) value, primarily for
@@ -3475,7 +3464,7 @@ def make_constant(value: Scalar) -> Future:
     return future
 
 
-def logical_not(value: bool) -> Future:
+def logical_not(value: bool) -> _Future:
     """Boolean negation.
 
     If the argument is a gmxapi compatible Data or Future object, a new View or
index db6133c2ed67342bc93efcaeaf6dd41aa4b954b7..669991bbaddfdee00ea53c505933286a43ccbcad 100644 (file)
@@ -99,7 +99,6 @@ void PyContext::addMDModule(pybind11::object force_object)
                     delete holder_ptr;
                     // \todo double-check whether there is something we should do to invalidate a PyCapsule.
                 }
-                ;
             };
         auto capsule = py::capsule(holder,
                                    gmxapi::MDHolder_Name,
index 8852b45fb4a2500fcece9dce8dc233bc8d001589..e5df27399c1b1c8eb654959d64dcf1392490d7d0 100644 (file)
@@ -223,7 +223,10 @@ def _get_mpi_ensemble_communicator(session_communicator, ensemble_size):
     rather than a null communicator in processes that aren't participating in
     a given ensemble.
     """
-    from mpi4py import MPI
+    try:
+        from mpi4py import MPI
+    except ImportError:
+        raise exceptions.FeatureNotAvailableError('MPI ensemble communicator requires mpi4py package.')
 
     session_size = session_communicator.Get_size()
     session_rank = session_communicator.Get_rank()
@@ -335,11 +338,14 @@ def _acquire_communicator(communicator=None):
 
     if communicator is None:
         try:
-            import mpi4py.MPI as MPI
-            communicator = MPI.COMM_WORLD
+            import mpi4py.MPI as _MPI
         except ImportError:
+            _MPI = None
+        if _MPI is None:
             logger.info("mpi4py is not available for default session communication.")
             communicator = _DummyCommunicator()
+        else:
+            communicator = _MPI.COMM_WORLD
     else:
         communicator = communicator
 
index 6d6f8ae628e19a3e21783079b7564c3c51dbf45c..f1790fa7dafda86971d0b27ef4dcf26602676bbf 100644 (file)
@@ -43,8 +43,8 @@ __all__ = ['TprFile', 'read_tpr', 'write_tpr_file']
 
 import os
 
+import gmxapi._gmxapi as _gmxapi
 from gmxapi import exceptions
-from gmxapi import _gmxapi
 
 
 class TprFile(object):
@@ -71,11 +71,6 @@ class TprFile(object):
         Note:
             Currently, TPR files are read-only from the Python interface.
 
-        Example:
-
-            >>> import gmxapi as gmx
-            >>> filehandle = gmx.TprFile(filename, 'r')
-
         """
         if filename is None:
             raise exceptions.UsageError("TprFile objects must be associated with a file.")
@@ -90,7 +85,7 @@ class TprFile(object):
         self._tprFileHandle = None
 
     def __repr__(self):
-        return "gmx.fileio.TprFile('{}', '{}')".format(self.filename, self.mode)
+        return "{}('{}', '{}')".format(self.__class__.__name__, self.filename, self.mode)
 
     def __enter__(self):
         self._tprFileHandle = _gmxapi.read_tprfile(self.filename)
index 67aad50c4ad3b3ca5392855351931bc5e15aab4b..b542e79d4e25c6640f6d7cdc5d0b68d11dff36fa 100644 (file)
@@ -333,6 +333,7 @@ class StandardDirector(gmxapi.abc.OperationDirector):
 
                 def simulation_input_workaround(input, parameters: dict):
                     source = input
+                    # TODO: Normalize mechanism for obtaining SimulationInput references.
                     if hasattr(source, 'output'):
                         source = input.output
                     assert hasattr(source, '_simulation_input')
@@ -380,6 +381,10 @@ def modify_input(input,
         # Allow automatic dispatching
         source_context = None
 
+    # TODO: Fix this protocol to do dispatching in the correct place.
+    # The source Context here is None (the handle Context). The resources themselves
+    # may be from different Contexts, so we should dispatch at the add_resource
+    # builder method, not here in the director client.
     resource_factory = node_director.resource_factory(source=source_context, target=target_context)
     resources = resource_factory(input=input, parameters=parameters)
     handle = node_director(resources=resources, label=label)
index d2338d4c4670c1823541eff0380e764670db5452..12458233bb628ce1f692afa27dc5dd2bcf936ed8 100644 (file)
@@ -38,24 +38,24 @@ Provides additional support for annotation and static type checking.
 Extends the specifications of the abstract base classes in gmxapi.abc.
 """
 
-from typing import TypeVar, Generic, NewType, Type, Callable
+from typing import TypeVar, Generic, NewType, Type
 
-import gmxapi as gmx
+import gmxapi.abc
 
 # Use SourceTypeVar and ResultTypeVar for static type hints, annotations, and as a parameter to generics.
 # Use valid_source_types and valid_result_types for run-time type checking.
-ResultTypeVar = TypeVar('ResultTypeVar', *(str, bool, int, float, dict, gmx.abc.NDArray))
+ResultTypeVar = TypeVar('ResultTypeVar', *(str, bool, int, float, dict, gmxapi.abc.NDArray))
 valid_result_types = ResultTypeVar.__constraints__
 
 SourceTypeVar = TypeVar('SourceTypeVar',
-                        *(str, bool, int, float, dict, gmx.abc.NDArray, gmx.abc.EnsembleDataSource))
+                        *(str, bool, int, float, dict, gmxapi.abc.NDArray, gmxapi.abc.EnsembleDataSource))
 valid_source_types = SourceTypeVar.__constraints__
 
 # Place holder for type annotations of Context objects.
 # TODO: Expand to support some static type checking.
-_Context = TypeVar('_Context', bound=gmx.abc.Context)
+_Context = TypeVar('_Context', bound=gmxapi.abc.Context)
 # Type variable that binds to subclasses of the forward referenced OperationImplentation ABC.
-_Op = TypeVar('_Op', bound=gmx.abc.OperationImplementation)
+_Op = TypeVar('_Op', bound=gmxapi.abc.OperationImplementation)
 
 # Note: We could make many types generic in SourceContext and TargetContext and
 # compose functionality into instances of a helpful base class with the option of
@@ -86,7 +86,7 @@ class Future(Generic[ResultTypeVar]):
         ...
 
 
-class OperationReference(gmx.abc.OperationReference, Generic[_Op]):
+class OperationReference(gmxapi.abc.OperationReference, Generic[_Op]):
     """Object with an OperationReference interface.
 
     Generic version of AbstractOperationReference, parameterized by operation
@@ -110,7 +110,7 @@ class OperationDirector(Generic[_Op, _Context]):
     """
 
 
-class OperationImplementation(Generic[_Op], gmx.abc.OperationImplementation):
+class OperationImplementation(Generic[_Op], gmxapi.abc.OperationImplementation):
     """Essential interface of an Operation implementation.
 
     Describe the essential features of an Operation that can be registered with