From 3e4bc8845d7b7336873b9c7faa110a5a5f6fa113 Mon Sep 17 00:00:00 2001
From: Keck Jean-Baptiste <jean-baptiste.keck@imag.fr>
Date: Sat, 3 Jun 2017 13:04:31 +0200
Subject: [PATCH] fields requirements fix

---
 .../taylor_green/taylor_green_monoscale.py    |  4 +-
 .../backend/device/opencl/opencl_operator.py  | 12 +--
 .../host/fortran/operator/diffusion.py        |  5 +-
 .../host/fortran/operator/fortran_fftw.py     | 20 +++-
 hysop/core/graph/computational_graph.py       | 45 ++++++---
 hysop/core/mpi/topology_descriptor.py         | 98 ++++++++++++-------
 hysop/fields/field_requirements.py            | 69 ++++++++++---
 hysop/tools/hash.py                           |  8 +-
 8 files changed, 184 insertions(+), 77 deletions(-)

diff --git a/examples/taylor_green/taylor_green_monoscale.py b/examples/taylor_green/taylor_green_monoscale.py
index c6812c832..e516e447d 100644
--- a/examples/taylor_green/taylor_green_monoscale.py
+++ b/examples/taylor_green/taylor_green_monoscale.py
@@ -85,8 +85,8 @@ def do_solve(npts, viscosity=1./1600., lcfl=0.125, cfl=0.50):
         
     problem = Problem()
     problem.insert(splitting)
-    # problem.insert(diffusion)
-    # problem.insert(poisson)
+    problem.insert(diffusion)
+    problem.insert(poisson)
     
     problem.build()
     # problem.display()
diff --git a/hysop/backend/device/opencl/opencl_operator.py b/hysop/backend/device/opencl/opencl_operator.py
index 39fb6708f..c5222da03 100644
--- a/hysop/backend/device/opencl/opencl_operator.py
+++ b/hysop/backend/device/opencl/opencl_operator.py
@@ -94,19 +94,19 @@ class OpenClOperator(ComputationalGraphOperator):
     @debug
     def handle_field_requirements(self):
         """
-        Called just after handle_method(), ie self.method has been set.
-        Topology requirements are:
+        called just after handle_method(), ie self.method has been set.
+        topology requirements are:
             1) min and max ghosts for each input and output variables
             2) allowed splitting directions for cartesian topologies
             3) required local and global transposition state, if any. 
             and more
-        They are stored in self.input_field_requirements and
+        they are stored in self.input_field_requirements and
         self.output_field_requirements.
 
-        Keys are continuous fields and values are of type
-        hysop.fields.field_requirement.DiscreteFieldRequirements
+        keys are continuous fields and values are of type
+        hysop.fields.field_requirement.discretefieldrequirements
 
-        Default is Backend.OPENCL, no min or max ghosts, Basis.CARTESIAN and no specific
+        default is backend.opencl, no min or max ghosts, basis.cartesian and no specific
         transposition state for each input and output variables.
         """
 
diff --git a/hysop/backend/host/fortran/operator/diffusion.py b/hysop/backend/host/fortran/operator/diffusion.py
index 8acddebd0..0d9e4cc31 100644
--- a/hysop/backend/host/fortran/operator/diffusion.py
+++ b/hysop/backend/host/fortran/operator/diffusion.py
@@ -3,7 +3,8 @@ from hysop.backend.host.fortran.operator.fortran_fftw import FortranFFTWOperator
 from hysop.tools.types       import check_instance, InstanceOf
 from hysop.tools.decorators  import debug
 from hysop.tools.numpywrappers import npw
-from hysop import Field, Cartesian
+from hysop.fields.continuous import Field
+from hysop.core.mpi.topology_descriptor import CartesianDescriptors
 
 class DiffusionFFTW(FortranFFTWOperator):
 
@@ -32,7 +33,7 @@ class DiffusionFFTW(FortranFFTWOperator):
 
         check_instance(input_field,  Field)
         check_instance(output_field, Field)
-        check_instance(variables, dict, keys=Field, values=Cartesian)
+        check_instance(variables, dict, keys=Field, values=CartesianDescriptors)
         
         assert variables[input_field] == variables[output_field], \
                 'input and output topology mismatch'
diff --git a/hysop/backend/host/fortran/operator/fortran_fftw.py b/hysop/backend/host/fortran/operator/fortran_fftw.py
index 18233ea59..f1eaa26f3 100644
--- a/hysop/backend/host/fortran/operator/fortran_fftw.py
+++ b/hysop/backend/host/fortran/operator/fortran_fftw.py
@@ -6,12 +6,13 @@ except ImportError:
     msg += 'Try to recompile HySoP with WITH_FFTW=ON'
     raise ImportError(msg)
 
-from hysop import Cartesian, Field
 from hysop.constants import HYSOP_ORDER
 from hysop.tools.numpywrappers import npw
 from hysop.tools.decorators import debug
 from hysop.tools.types import check_instance
 from hysop.core.graph.computational_operator import ComputationalGraphOperator
+from hysop.core.mpi.topology_descriptor import CartesianDescriptors
+from hysop.fields.continuous import Field
 
 class FortranFFTWOperator(ComputationalGraphOperator):
 
@@ -20,8 +21,8 @@ class FortranFFTWOperator(ComputationalGraphOperator):
         super(FortranFFTWOperator, self).__init__(input_vars=input_vars, 
                 output_vars=output_vars, **kwds)
         
-        check_instance(input_vars, dict,  keys=Field, values=Cartesian)
-        check_instance(output_vars, dict, keys=Field, values=Cartesian)
+        check_instance(input_vars, dict,  keys=Field, values=CartesianDescriptors)
+        check_instance(output_vars, dict, keys=Field, values=CartesianDescriptors)
         
         domain   = self.input_vars.keys()[0].domain
         topology = self.input_vars.values()[0]
@@ -39,6 +40,19 @@ class FortranFFTWOperator(ComputationalGraphOperator):
     def initialize(self,topgraph_method=None):
         super(FortranFFTWOperator,self).initialize(topgraph_method)
     
+    @debug
+    def handle_field_requirements(self):
+        super(FortranFFTWOperator, self).handle_field_requirements()
+        
+        # set can_split set to True in all directions except the contiguous one
+        for field, req in self.input_field_requirements.iteritems():
+            can_split = req.can_split
+            can_split[0] = False
+            req.can_split  = can_split
+            req.min_ghosts = npw.zeros_like(req.min_ghosts)
+            req.max_ghosts = npw.zeros_like(req.min_ghosts)
+
+    
     @debug
     def discretize(self):
         if self.discretized:
diff --git a/hysop/core/graph/computational_graph.py b/hysop/core/graph/computational_graph.py
index de99a8590..d5455a4bb 100644
--- a/hysop/core/graph/computational_graph.py
+++ b/hysop/core/graph/computational_graph.py
@@ -74,28 +74,50 @@ class ComputationalGraph(ComputationalGraphNode):
             ss += '\n Backend {}:'.format(backend.kind)
             ss += '\n  *'+'\n  *'.join( [print_topo(t) 
                 for t in sorted(topologies,key=lambda x: x.get_id())] )
-        ss += '\n'
+        ss += '\n{}\n'.format(len('== ComputationalGraph {} topology report =='.format(self.name))*'=')
         return ss
 
     @initialized
     def field_requirements_report(self):
         
         inputs = {}
+        sinputs  = ''
         for field, mreqs in self.input_field_requirements.iteritems():
             for td, reqs in mreqs.requirements.iteritems():
-                print td==mreqs.requirements.keys()[0]
+                for req in reqs:
+                    if __DEBUG__ or (__VERBOSE__ and not req.is_default()):
+                        inputs.setdefault(td, {}).setdefault(field, []).append(req)
+        for td, td_reqs in inputs.iteritems():
+            sinputs += '\n {}'.format(td)
+            for field, reqs in td_reqs.iteritems():
+                for req in reqs:
+                    sinputs += '\n  *{}'.format(req)
+
+        outputs = {}
+        soutputs  = ''
+        for field, mreqs in self.output_field_requirements.iteritems():
+            for td, reqs in mreqs.requirements.iteritems():
+                for req in reqs:
+                    if __DEBUG__ or (__VERBOSE__ and not req.is_default()):
+                        outputs.setdefault(td, {}).setdefault(field, []).append(req)
+        for td, td_reqs in outputs.iteritems():
+            soutputs += '\n {}'.format(td)
+            for field, reqs in td_reqs.iteritems():
+                for req in reqs:
+                    soutputs += '\n  *{}'.format(req)
 
-        inputs  = ''
-        outputs = ''
-        
         ss = '== ComputationalGraph {} field requirements report =='.format(self.name)
-        ss+= '\n>INPUTS:'
-        ss+= '\n{}'.format(inputs)
-        ss+= '\n>OUTPUTS:'
-        ss+= '\n{}'.format(outputs)
-        ss += len('== ComputationalGraph {} field requirements report =='.format(self.name))*'='
+        if sinputs:
+            ss+= '\n>INPUTS:{}'.format(sinputs)
+        else:
+            ss+= '\n>INPUTS: None'
+        if soutputs:
+            ss+= '\n>OUTPUTS:{}'.format(soutputs)
+        else:
+            ss+= '\n>OUTPUTS: None'
+        ss += '\n'+len('== ComputationalGraph {} field requirements report =='.format(self.name))*'='+'\n'
         return ss
-    
+
     @debug
     @not_initialized
     def push_nodes(self, *args):
@@ -510,7 +532,6 @@ class ComputationalGraph(ComputationalGraphNode):
                     display_props=vertex_info,
                     display_props_size=14)
     
-    
     @debug
     @graph_built
     def discretize(self):
diff --git a/hysop/core/mpi/topology_descriptor.py b/hysop/core/mpi/topology_descriptor.py
index 4dabef787..5af45499c 100644
--- a/hysop/core/mpi/topology_descriptor.py
+++ b/hysop/core/mpi/topology_descriptor.py
@@ -1,6 +1,6 @@
 
 from abc import ABCMeta, abstractmethod
-from hysop.deps import np, hashlib
+from hysop.deps import np, hashlib, copy
 from hysop.tools.types import check_instance
 from hysop.tools.numpywrappers import npw
 from hysop.constants import Backend, BoundaryCondition
@@ -12,16 +12,32 @@ from hysop.domain.mesh import Mesh
 class TopologyDescriptor(object):
     """
     Describes how a topology should be built.
-    kargs allows for backend specific variables.
+    kwds allows for backend specific variables.
+
+    TopologyDescriptor is immutable.
     """
     __metaclass__ = ABCMeta
 
-    def __init__(self, mpi_params, domain, backend, **kargs):
+    def __init__(self, mpi_params, domain, backend, **kwds):
         super(TopologyDescriptor, self).__init__()
-        self.mpi_params = mpi_params
-        self.domain = domain
-        self.backend=backend
-        self.extra_kargs = kargs
+        self._mpi_params = mpi_params
+        self._domain = domain
+        self._backend=backend
+        self._extra_kwds = frozenset(kwds.items())
+
+    def get_mpi_params(self):
+        return self._mpi_params
+    def get_domain(self):
+        return self._domain
+    def get_backend(self):
+        return self._backend
+    def get_extra_kwds(self):
+        return dict(self._extra_kwds)
+        
+    mpi_params  = property(get_mpi_params)
+    domain      = property(get_domain)
+    backend     = property(get_backend)
+    extra_kwds  = property(get_extra_kwds)
 
     def __eq__(self, other):
         if not isinstance(other, TopologyDescriptor):
@@ -29,17 +45,17 @@ class TopologyDescriptor(object):
         eq  = (self.mpi_params  == other.mpi_params)
         eq &= (self.domain      == other.domain)
         eq &= (self.backend     == other.backend)
-        eq &= (self.extra_kargs == other.extra_kargs)
+        eq &= (self.extra_kwds  == other.extra_kwds)
         return eq
-    
+
     def __ne__(self, other):
         return not self.__eq__(other)
-    
+
     def __hash__(self):
-        return hash((self.mpi_params, self.domain, self.backend, self.extra_kargs))
-    
+        return hash((self._mpi_params, self._domain, self._backend, self._extra_kwds))
+
     @staticmethod
-    def build_descriptor(backend, operator, field, handle, **kargs):
+    def build_descriptor(backend, operator, field, handle, **kwds):
         if isinstance(handle, Topology):
             # handle is already a Topology, so we return it.
             return handle
@@ -48,13 +64,13 @@ class TopologyDescriptor(object):
             return handle
         elif isinstance(handle, CartesianDescriptors):
             return CartesianDescriptor.build_descriptor(backend, operator, 
-                    field, handle, **kargs)
+                    field, handle, **kwds)
         else:
             msg='Unknown handle of class {} to build a TopologyDescriptor.'
             msg=msg.format(handle.__class__)
             raise TypeError(msg)
     
-    def choose_or_create_topology(self, known_topologies, **kargs):
+    def choose_or_create_topology(self, known_topologies, **kwds):
         """
         Returns a topology that is either taken from known_topologies, a set
         of user specified topologies which are ensured to be compatible
@@ -62,16 +78,16 @@ class TopologyDescriptor(object):
         if choose_topology() returns None.
         """
         check_instance(known_topologies, list, values=Topology)
-        topo = self.choose_topology(known_topologies, **kargs) or \
-               self.create_topology(**kargs)
+        topo = self.choose_topology(known_topologies, **kwds) or \
+               self.create_topology(**kwds)
         return topo
     
     @abstractmethod
-    def choose_topology(self, known_topologies, **kargs):
+    def choose_topology(self, known_topologies, **kwds):
         pass
 
     @abstractmethod
-    def create_topology(self, **kargs):
+    def create_topology(self, **kwds):
         pass
 
     def dim(self):
@@ -91,33 +107,47 @@ class TopologyDescriptor(object):
 class CartesianDescriptor(TopologyDescriptor):
     """
     Describes how a Cartesian topology should be built.
-    kargs allows for backend specific variables.
+    kwds allows for backend specific variables.
+    
+    CartesianDescriptor is immutable.
     """
-    def __init__(self, mpi_params, domain, backend, discretization, **kargs):
+    def __init__(self, mpi_params, domain, backend, discretization, **kwds):
         super(CartesianDescriptor, self).__init__(mpi_params=mpi_params, 
-                domain=domain, backend=backend, **kargs)
+                domain=domain, backend=backend, **kwds)
         # boundaries are stored in domain
-        self.discretization = discretization
-        self.mesh = self._global_mesh(domain, discretization)
-    
+        discretization = copy.copy(discretization)
+        discretization.resolution.flags.writeable = False
+        discretization.ghosts.flags.writeable = False
+
+        self._discretization = discretization
+        self._mesh = self._global_mesh(domain, discretization)
+
+    def get_discretization(self):
+        return self._discretization
+    def get_mesh(self):
+        return self._mesh
+
+    discretization = property(get_discretization)
+    mesh           = property(get_mesh)
+
     def __eq__(self, other):
         if not isinstance(other, CartesianDescriptor):
             return NotImplemented
         eq  = super(CartesianDescriptor, self).__eq__(other)
         eq &= (self.discretization == other.discretization)
-        print 'eqlol {}'.format(eq)
         return eq
 
     def __hash__(self):
-        print 'hashlol {}'.format(hash((super(CartesianDescriptor,self), self.discretization)))
-        return hash((super(CartesianDescriptor,self), self.discretization))
+        # hash(super(...)) does not work as expected so be call __hash__ directly
+        return super(CartesianDescriptor,self).__hash__() ^ hash(self._discretization)
 
     def __str__(self):
         return ':CartesianDescriptor: backend={}, resolution={}, domain={}'.format(
-                self.backend, self.discretization.resolution, self.domain.short_description())
+                    self.backend, self.discretization.resolution, 
+                    self.domain.short_description())
 
     @staticmethod
-    def build_descriptor(backend, operator, field, handle, **kargs):
+    def build_descriptor(backend, operator, field, handle, **kwds):
         from hysop.core.graph.computational_operator import ComputationalGraphOperator
         check_instance(backend, Backend)
         check_instance(operator, ComputationalGraphOperator)
@@ -131,21 +161,21 @@ class CartesianDescriptor(TopologyDescriptor):
         if isinstance(handle, Discretization):
             if handle.ghosts.sum() > 0:
                 raise ValueError(msg)
-            msg='mpi_params has not been set in operator {}.'.format(operator.name)
             if not hasattr(operator, 'mpi_params'):
+                msg='mpi_params has not been set in operator {}.'.format(operator.name)
                 raise RuntimeError(msg)
             return CartesianDescriptor(
                     backend=backend, 
                     domain=field.domain, 
                     mpi_params=operator.mpi_params,
                     discretization=handle,
-                    **kargs)
+                    **kwds)
         elif isinstance(handle, CartesianDescriptor):
             if handle.discretization.ghosts.sum() > 0:
                 raise ValueError(msg)
             return handle
         else:
-            # handle is a Cartesian instance, ghosts can be imposed by user here
+            # handle is a Cartesian instance, ghosts can be imposed freely by user here
             return handle
     
     def _global_mesh(self, domain, discretization):
@@ -166,7 +196,7 @@ class CartesianDescriptor(TopologyDescriptor):
 
         return mesh
     
-    def choose_topology(self, known_topologies, **kargs):
+    def choose_topology(self, known_topologies, **kwds):
         """
         Find optimal topology parameters from known_topologies.
         If None is returned, create_topology will be called instead.
diff --git a/hysop/fields/field_requirements.py b/hysop/fields/field_requirements.py
index 0eb651bc1..04e82fe13 100644
--- a/hysop/fields/field_requirements.py
+++ b/hysop/fields/field_requirements.py
@@ -1,4 +1,5 @@
 
+from hysop import __DEBUG__
 from hysop.deps import np, it
 from hysop.constants import Basis, transposition_states
 from hysop.tools.types import to_list, check_instance
@@ -17,14 +18,31 @@ def can_transpose_inplace(ltranspose, rtranspose):
 
 
 class DiscreteFieldRequirements(object):
+    
+    _registered_requirements = set()
+
     def __init__(self, operator, variables, field, 
             min_ghosts=None, 
             max_ghosts=None,
             can_split=None, 
             required_basis=None,
             required_transposition_state=None,
+            _register=True,
             **kwds):
 
+        if _register:
+            key = (id(operator), id(variables), id(field))
+            if key in self._registered_requirements:
+                msg='Operator {} has already registered requirements for field {} to variables id {}.'
+                msg=msg.format(operator.name, field.name, id(variables))
+                raise RuntimeError(msg)
+            else:
+                if __DEBUG__:
+                    msg='Operator {} registered requirements of field {} to variables id {}.'
+                    msg=msg.format(operator.name, field.name, id(variables))
+                    print msg
+                self._registered_requirements.update(key)
+
         super(DiscreteFieldRequirements, self).__init__(**kwds)
         
         check_instance(operator, ComputationalGraphNode)
@@ -43,9 +61,31 @@ class DiscreteFieldRequirements(object):
         self.required_basis = required_basis
         self.required_transposition_state = required_transposition_state
 
+    def is_default(self):
+        return (self == self._default())
+
+    def _default(self):
+        return DiscreteFieldRequirements(self._operator, self._variables, self._field, _register=False)
+
+    def __eq__(self, other):
+        eq  = (self.operator  is other.operator)
+        eq &= (self.variables is other.variables)
+        eq &= (self.field     is other.field)
+        eq &= (self.min_ghosts == other.min_ghosts).all()
+        eq &= (self.max_ghosts == other.max_ghosts).all()
+        eq &= (self.required_basis == other.required_basis)
+        eq &= (self.required_transposition_state == other.required_transposition_state)
+        return eq
+
     def __str__(self):
-        return '{}::{}  {}<=ghosts<{}  can_split={}  basis={}  tstate={}'.format(
-                self.operator.name, self.field.name, self.min_ghosts, self.max_ghosts+1,
+        def arraystr(array):
+            inf = u'+\u221e'
+            vals = [u''+str(x) if np.isfinite(x) else inf for x in array]
+            return u'[{}]'.format(u','.join(vals)).encode('utf-8').strip()
+
+        return '{:15s} {:>10s}<=ghosts<{:<10s}  can_split={}  basis={}  tstate={}'.format(
+                '{}::{}'.format(self.operator.name, self.field.name), 
+                arraystr(self.min_ghosts), arraystr(self.max_ghosts+1),
                 self.can_split.view(np.int8), 
                 '[{}]'.format(','.join(str(basis) for basis in self.required_basis)), 
                 self.required_transposition_state)
@@ -60,7 +100,7 @@ class DiscreteFieldRequirements(object):
         return self._required_basis
     def set_required_basis(self, basis):
         check_instance(basis, list, values=Basis, allow_none=True)
-        self._required_basis = basis or [Basis.NATURAL]*self.workdim
+        self._required_basis = basis or (Basis.NATURAL,)*self.workdim
 
     def get_min_ghosts(self):
         return self._min_ghosts
@@ -197,18 +237,15 @@ class MultiFieldRequirements(object):
         return sum(len(lreqs) for lreqs in self.requirements.values())
 
     def update(self, *update_reqs):
-        for reqs in update_reqs:
-            if isinstance(reqs, MultiFieldRequirements):
-                reqs = reqs.requirements.values()
+        for update_req in update_reqs:
+            if isinstance(update_req, MultiFieldRequirements):
+                tds  = update_req.requirements.keys()
+                reqs = update_req.requirements.values()
             else:
-                reqs = [[reqs]]
-            for lreq in reqs:
-                for req in lreq:
-                    check_instance(req, DiscreteFieldRequirements)
-                    assert req.field == self.field
-                    td_reqs = self.requirements.setdefault(req.topology_descriptor, [])
-                    if all((td_req.variables[td_req.field] is not req.variables[req.field]) for td_req in td_reqs):
-                        td_reqs.append(req)
+                tds  = [update_req.topology_descriptor]
+                reqs = [[update_req]]
+            for td, reqs in zip(tds, reqs):
+                td_reqs = self.requirements.setdefault(td, set()).update(reqs)
     
     def build_topologies(self):
         if self.built:
@@ -218,7 +255,7 @@ class MultiFieldRequirements(object):
         self.built = True
     
     def all_compatible(self):
-        for topology_descriptor in self.requirements.keys():
+        for topology_descriptor in self.requirements:
             requirements = self.requirements[topology_descriptor]
             assert len(requirements)>0
             for req0, req1 in it.combinations(requirements, 2):
@@ -240,7 +277,7 @@ class MultiFieldRequirements(object):
                     new_group = MultiFieldRequirements(self.field)
                     new_group.update(req)
                     sub_field_requirements.append(new_group)
-        # assert self.nrequirements() == sum(sf.nrequirements() for sf in sub_field_requirements)
+        assert self.nrequirements() == sum(sf.nrequirements() for sf in sub_field_requirements)
         return sub_field_requirements
 
     def _build_compatible_topologies(self):
diff --git a/hysop/tools/hash.py b/hysop/tools/hash.py
index 3d25dd79f..f45404240 100644
--- a/hysop/tools/hash.py
+++ b/hysop/tools/hash.py
@@ -1,4 +1,5 @@
 
+from hysop.deps import np, hashlib
 
 def hash_communicator(comm, h=None):
     from hysop.core.mpi import processor_hash
@@ -10,6 +11,9 @@ def hash_communicator(comm, h=None):
     else:
         return s
 
-def hash_nd_array(array, h):
-    return h.update(array.view(np.uint8))
+def hash_nd_array(array, h=None):
+    if h is None:
+        h = hashlib.sha1()
+    h.update(array.view(np.uint8))
+    return h.hexdigest()
 
-- 
GitLab