Implement slicing for LB #4178

KaiserMartin · 2021-03-25T14:46:03Z

Description of changes:

Modified getitem to detect slices in input keys
Introduced LBSlice class that calculates node indices from slice input and calls single node getter from LBFLuidRoutines class for every node in the slice.

The output is a multidimensional array containing the requested quantity and is of dimension len(slice_x)*len(slice_y)*len(slice_z)*len(quantity), where len(quantity) is the dimensions of the quantity, for example 3 in case of velocity. The output array is arranged in the way numpy cycles through slices. Therefore, when setting attributes one has to set the dimensions of the input array exactly the same way numpy cycles through the sliced nodes.
An error is issued if the dimensions of the slices and the input does not match.

src/python/espressomd/lb.pyx

KaiserMartin

Removing unnecessary c variables from population and squeezing output from property boundary so it has the right dimensions

KaiSzuttor · 2021-03-31T13:30:26Z

src/python/espressomd/lb.pyx

+    def __init__(self, key):
+        shape = lb_lbfluid_get_shape()
+        self.x_indices, self.y_indices, self.z_indices = self.get_indices(
+            key, shape[0], shape[1], shape[2])


Suggested change

def __init__(self, key):

shape = lb_lbfluid_get_shape()

self.x_indices, self.y_indices, self.z_indices = self.get_indices(

key, shape[0], shape[1], shape[2])

def __init__(self, key, shape):

self.shape = shape

self.x_indices, self.y_indices, self.z_indices = self.get_indices(

key, shape[0], shape[1], shape[2])

KaiSzuttor · 2021-03-31T13:30:46Z

src/python/espressomd/lb.pyx

@@ -542,3 +548,137 @@ cdef class LBFluidRoutines:

        def __set__(self, value):
            raise NotImplementedError
+
+cdef class LBSlice:


Suggested change

cdef class LBSlice:

class LBSlice:

KaiSzuttor · 2021-03-31T13:30:58Z

src/python/espressomd/lb.pyx

+    cdef np.ndarray x_indices
+    cdef np.ndarray y_indices
+    cdef np.ndarray z_indices


Suggested change

cdef np.ndarray x_indices

cdef np.ndarray y_indices

cdef np.ndarray z_indices

KaiSzuttor · 2021-03-31T13:31:32Z

src/python/espressomd/lb.pyx

+
+    property density:
+        def __get__(self):
+            prop_name = "density"


Suggested change

prop_name = "density"

KaiSzuttor · 2021-03-31T13:31:41Z

src/python/espressomd/lb.pyx

+                self.x_indices, self.y_indices, self.z_indices, prop_name, shape_res), axis=3)
+
+        def __set__(self, value):
+            prop_name = "density"


Suggested change

prop_name = "density"

KaiSzuttor · 2021-03-31T13:32:24Z

src/python/espressomd/lb.pyx

+
+    property pressure_tensor_neq:
+        def __get__(self):
+            prop_name = "pressure_tensor_neq"


Suggested change

prop_name = "pressure_tensor_neq"

KaiSzuttor · 2021-03-31T13:32:32Z

src/python/espressomd/lb.pyx

+
+    property population:
+        def __get__(self):
+            prop_name = "population"


Suggested change

prop_name = "population"

KaiSzuttor · 2021-03-31T13:32:39Z

src/python/espressomd/lb.pyx

+                self.x_indices, self.y_indices, self.z_indices, prop_name, shape_res)
+
+        def __set__(self, value):
+            prop_name = "population"


Suggested change

prop_name = "population"

KaiSzuttor · 2021-03-31T13:32:48Z

src/python/espressomd/lb.pyx

+    property boundary:
+        def __get__(self):
+            shape_res = (1,)
+            prop_name = "boundary"


Suggested change

prop_name = "boundary"

KaiSzuttor · 2021-03-31T13:35:29Z

src/python/espressomd/lb.pyx

+    property density:
+        def __get__(self):
+            prop_name = "density"
+            shape_res = (1,)
+            return np.squeeze(self.get_values(
+                self.x_indices, self.y_indices, self.z_indices, prop_name, shape_res), axis=3)
+
+        def __set__(self, value):
+            prop_name = "density"
+            self.set_values(
+                self.x_indices,
+                self.y_indices,
+                self.z_indices,
+                prop_name,
+                value)
+


Suggested change

property density:

def __get__(self):

prop_name = "density"

shape_res = (1,)

return np.squeeze(self.get_values(

self.x_indices, self.y_indices, self.z_indices, prop_name, shape_res), axis=3)

def __set__(self, value):

prop_name = "density"

self.set_values(

self.x_indices,

self.y_indices,

self.z_indices,

prop_name,

value)

@property

def density(self):

prop_name = "density"

shape_res = (1,)

return np.squeeze(self.get_values(

self.x_indices, self.y_indices, self.z_indices, prop_name, shape_res), axis=3)

@density.setter

def density(self, value):

self.set_values(

self.x_indices,

self.y_indices,

self.z_indices,

prop_name,

value)

this is the modern python syntax for properties

jngrad · 2021-03-31T14:06:12Z

@KaiserMartin Here is the result of applying Kai's review: jngrad/espresso@fix-4143 (diff view)

jngrad

PR ready for merge from my side.

jngrad · 2021-03-31T18:57:13Z

please don't merge it yet, we're looking into a simplification using a python reflection

jngrad · 2021-03-31T20:53:43Z

The LBSlice now returns numpy arrays as array_locked. Properties are generated automatically.
The broken LBSlice.boundary raise assertion check is now fixed.

jngrad · 2021-03-31T21:12:54Z

I accidentally pushed the python branch instead of fix-4143 to KaiserMartin's fork, and now GitHub rejects further pushes:

$ git push -f KaiserMartin fix-4143:python
X11 forwarding request failed on channel 0
Enumerating objects: 99, done.
Counting objects: 100% (99/99), done.
Delta compression using up to 32 threads
Compressing objects: 100% (88/88), done.
Writing objects: 100% (88/88), 17.57 KiB | 1.46 MiB/s, done.
Total 88 (delta 71), reused 0 (delta 0)
remote: Resolving deltas: 100% (71/71), completed with 11 local objects.
To github.com:KaiserMartin/espresso.git
 ! [remote rejected]     fix-4143 -> python (permission denied)
error: failed to push some refs to 'git@github.com:KaiserMartin/espresso.git'

GitHub also closed the PR using my username. Moving over to #4195. Sorry for the inconvenience.

KaiSzuttor reviewed Mar 31, 2021

View reviewed changes

src/python/espressomd/lb.pyx Outdated Show resolved Hide resolved

src/python/espressomd/lb.pyx Outdated Show resolved Hide resolved

KaiSzuttor reviewed Mar 31, 2021

View reviewed changes

src/python/espressomd/lb.pyx Outdated Show resolved Hide resolved

KaiserMartin commented Mar 31, 2021

View reviewed changes

KaiSzuttor reviewed Mar 31, 2021

View reviewed changes

jngrad reviewed Mar 31, 2021

View reviewed changes

jngrad requested a review from KaiSzuttor March 31, 2021 15:55

jngrad added New Feature Python labels Mar 31, 2021

jngrad added this to the Espresso 4.2 milestone Mar 31, 2021

RudolfWeeber previously approved these changes Mar 31, 2021

View reviewed changes

RudolfWeeber added ApiChange automerge Merge with kodiak Improvement labels Mar 31, 2021

jngrad removed the automerge Merge with kodiak label Mar 31, 2021

jngrad dismissed RudolfWeeber’s stale review via e41e9df March 31, 2021 20:46

jngrad closed this Mar 31, 2021

jngrad force-pushed the python branch from e41e9df to 051c5b8 Compare March 31, 2021 21:01

jngrad removed this from the Espresso 4.2 milestone Jun 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement slicing for LB #4178

Implement slicing for LB #4178

KaiserMartin commented Mar 25, 2021

KaiserMartin left a comment

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

KaiSzuttor Mar 31, 2021

jngrad commented Mar 31, 2021

jngrad left a comment

jngrad commented Mar 31, 2021

jngrad commented Mar 31, 2021

jngrad commented Mar 31, 2021

	cdef np.ndarray x_indices
	cdef np.ndarray y_indices
	cdef np.ndarray z_indices

Implement slicing for LB #4178

Implement slicing for LB #4178

Conversation

KaiserMartin commented Mar 25, 2021

KaiserMartin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jngrad commented Mar 31, 2021

jngrad left a comment

Choose a reason for hiding this comment

jngrad commented Mar 31, 2021

jngrad commented Mar 31, 2021

jngrad commented Mar 31, 2021