Start PetscObject #1

ZoeLeibowitz · 2023-06-20T11:10:06Z

Playing around with PR UI.

ZoeLeibowitz · 2023-06-23T13:03:37Z

Start of PetscObject ... please review if you have time, thanks!

mloubout · 2023-06-23T15:07:11Z

@edsml-zl5621 @FabioLuporini I think can make use of the standard devito objects and interface rather than resulting in all these new classe. Below is an example of something that would work as a nice extension without needing to go all the way down to object but might need some tweaks

from ctypes import POINTER

import numpy as np

from devito import *

from devito.ir.iet import Definition, DummyExpr
from devito.types.dense import DiscreteFunction
from devito.types.basic import Basic, AbstractSymbol
from devito.types.object import AbstractObject
from devito.tools import dtype_to_cstr, dtype_to_ctype


class PetscObject(AbstractSymbol):
    is_PetscObject = True
    """
    PetscObjects encode their dtype as a class attribute.
    """

    @property
    def is_const(self):
        return self._is_const
    
    @property
    def dtype(self):
        return self._dtype

    @property
    def _C_name(self):
        return self.name

    @property
    def _C_ctype(self):
        name = dtype_to_cstr(self._dtype).capitalize()
        return type('Petsc%s' % name, (dtype_to_ctype(self._dtype),), {})


class PetscFunction(Function):

    @property
    def _C_ctype(self):
        ctypename = 'Petsc%s' % dtype_to_cstr(self.dtype).capitalize()
        ctype = dtype_to_ctype(self.dtype)
        r = POINTER(type(ctypename, (ctype,), {}))
        for n in range(len(self.dimensions)-1):
            r = POINTER(r)
        return r


grid = Grid((2, 2))
x, y = grid.dimensions
# pointer and const functionality
ptr1 = PetscFunction(name='ptr1', dtype=np.int32, grid=grid, dimensions=(x,), shape=(2,), is_const=True)
ptr2 = PetscFunction(name='ptr2', dtype=np.int32, grid=grid, dimensions=(x,), shape=(2,))
pptr = PetscFunction(name='pptr', dtype=np.int32, grid=grid)

defn1 = Definition(ptr1)
defn2 = Definition(ptr2)
defn3 = Definition(pptr)

# algebraic expressions
x = PetscObject(name='x', dtype=np.int32)
y = PetscObject(name='y', dtype=np.int32)

expr1 = DummyExpr(x, 15, init=True)
expr2 = DummyExpr(x, y-1)

print(str(defn1))
print(str(defn2))
print(str(defn3))
print(str(expr1))
print(str(expr2))

assert str(defn1) == "const PetscInt * ptr1;"
assert str(defn2) == "PetscInt * ptr2;"
assert str(defn3) == "PetscInt ** pptr;"
assert str(expr1) == "PetscInt x = 15;"
assert str(expr2) == "x = -1 + y;"

FabioLuporini · 2023-06-29T06:37:23Z

devito/types/object.py

+    # taken exactly from Function class but removed the "shape_global"
+    #  # and also added the option for all dimensions, shape and grid to be None
+    @classmethod
+    def __shape_setup__(cls, **kwargs):


both shape_setup and indices_setup should belong to AbstractObjectWithShape

Does this mean I should remove the 'empty' shape_setup and indices_setup that are currently inside AbstractObjectWithShape and replace them with these ones?

Why not?

Say I create a new subtype, let's call it class MyNewObject(AbstractObjectWithShape)

I want this new type to share all the key PetscObject mechanisms , don't I?

FabioLuporini · 2023-06-29T06:37:57Z

devito/types/object.py

+            if len(shape) != len(dimensions):
+                raise ValueError("`shape` and `dimensions` must have the "
+                                 "same number of entries")
+            loc_shape = []


no distinction between global and local shape (MPI irrelevant for Objects), so we don't need this part

FabioLuporini · 2023-06-29T06:38:24Z

devito/types/object.py

+                              PetscObject
+    """
+
+    AbstractObjectWithShape = True


don't add unless proven useful. Too early

FabioLuporini · 2023-06-29T06:40:05Z

devito/types/object.py

+
+                         AbstractObjectWithShape
+                                   |
+                              PetscObject


a PetscObject is way too special to live inside the types/object directory. Only the more general class belong here.

The PetscObject type should instead be defined wherever you will implement the petsc machinery (passes/iet/petsc/ ?) . Just like we do for some MPI Objects under /mpi/distributed or /mpi/routines

EdCaunt · 2023-06-30T08:08:15Z

devito/types/object.py

@@ -1,14 +1,17 @@
 from ctypes import byref
+from ctypes import POINTER


Why not from ctypes import byref, POINTER?

EdCaunt · 2023-06-30T08:16:28Z

devito/types/object.py

+        self._is_const = kwargs.get('is_const', False)
+
+    # taken exactly from Function class but removed the "shape_global"
+    #  # and also added the option for all dimensions, shape and grid to be None


Extra # in the second comment here

EdCaunt · 2023-06-30T08:22:11Z

devito/types/object.py

+    @classmethod
+    def __indices_setup__(cls, **kwargs):
+        grid = kwargs.get('grid')
+        # shape = kwargs.get('shape', None)


Leftover comment

EdCaunt · 2023-06-30T08:25:52Z

devito/types/object.py

+
+    @property
+    def _C_ctype(self):
+        """


Is the empty docstring intentional?

ZoeLeibowitz · 2023-08-03T15:53:02Z

Hi, I added a FunctionPtr class and a test for the symbolic representation of C's function pointers. I'm not sure if this is the right way to do it so if someone could have a look that would be great! Thank you

FabioLuporini · 2023-08-04T10:06:01Z

devito/symbolics/extended_sympy.py

+    Symbolic representation of C's function pointers.
+    """
+
+    _return_typ = ''


why aren't these instance attributes?

also, you can add an 'e' at the end of 'type' ;)

I took inspo from the Cast class https://github.com/devitocodes/devito/blob/0abe3952a792ed9ba5bf9ef904fa2fec04789a79/devito/symbolics/extended_sympy.py#L370C1-L400C33 and _base_typ is not an instance attribute. Is there a particular reason for this?

I think it's a leftover from code that has been refactorer multiple times. You may probably tweak that too !

FabioLuporini · 2023-08-04T10:06:36Z

devito/symbolics/extended_sympy.py

+    func = Pickable._rebuild
+
+    @property
+    def _op(self):


shouldn't this just be __str__ (== __repr__)

FabioLuporini · 2023-08-04T10:08:47Z

Also, how about a first tiny tiny PR that only ships FunctionPointer and related tests -- see:

test_symbolics.py
test_pickling.py

THe first merge into devito master is big milestone :-)

mloubout · 2023-08-04T14:59:41Z

devito/types/object.py

+    # need to check this?
+    # __rkwargs__ = ('name', 'dtype', 'is_const', 'grid', 'shape', 'dimensions')
+
+    def __new__(cls, *args, **kwargs):


This still looks like there is a quite large amount of overlap with AbstractFunction so I'm not sure why this is needed.

Hi, I forgot to ask about this in the meeting. @FabioLuporini do you have any comments on this, because I am unsure?

@mloubout AbstractObject doesn't inherit from AbstractFunction, so unless we lift some of the construction logic to the closest common ancestor (as in, Basic), this needs to stay here, I think

But this doesn't need to inheriting from AbstractObject if there is already an existing type that does all of it. This just redefines AbstractFunction with a new calss name and _const it should be a 2 liners inheriting from AbstractFunction

mloubout · 2023-08-09T12:03:57Z

devito/passes/iet/petsc.py

+    # __rkwargs__ = (AbstractObjectWithShape.__rkwargs__ +
+    #                ('petsc_type',))
+
+    def __init__(self, name, petsc_type, **kwargs):


Should implement _init_finalize_ not __init__ to follow AbstractObjectWithShape construction

EdCaunt · 2023-08-09T13:34:35Z

devito/symbolics/extended_sympy.py

+        return obj
+
+    def __str__(self):
+        return "(%s (%s)(%s))%s" % (self.return_type, '*',


Could this be simplified to return "(%s (*)(%s))%s" % (self.return_type, self.parameter_type, self.func_name)?

EdCaunt · 2023-08-09T13:35:09Z

devito/tools/dtypes_lowering.py

+
+# this probably shouldn't be in this file
+def petsc_type_to_ctype(petsc_type):
+    "Map Petsc types to ctypes type"


Triple quotes on the docstring

EdCaunt · 2023-08-09T13:35:40Z

devito/symbolics/extended_sympy.py

@@ -400,6 +401,37 @@ def _op(self):
        return '(%s)' % self.typ


+class FunctionPointer(sympy.Expr, Pickable):
+


Remove this whitespace?

…ct itself

ZoeLeibowitz added the testing Playing around. Don't review. label Jun 20, 2023

ZoeLeibowitz changed the title ~~Test.~~ Start PetscObject Jun 23, 2023

ZoeLeibowitz added enhancement New feature or request and removed testing Playing around. Don't review. labels Jun 23, 2023

ZoeLeibowitz self-assigned this Jun 23, 2023

ZoeLeibowitz requested review from mloubout and georgebisbas June 23, 2023 13:14

ZoeLeibowitz requested a review from FabioLuporini June 28, 2023 11:37

FabioLuporini reviewed Jun 29, 2023

View reviewed changes

EdCaunt reviewed Jun 30, 2023

View reviewed changes

ZoeLeibowitz requested review from EdCaunt and FabioLuporini August 3, 2023 15:54

FabioLuporini reviewed Aug 4, 2023

View reviewed changes

ZoeLeibowitz closed this Aug 4, 2023

ZoeLeibowitz reopened this Aug 4, 2023

mloubout reviewed Aug 4, 2023

View reviewed changes

ZoeLeibowitz force-pushed the petsc_object branch from 9d73c8e to a9fd0c2 Compare August 8, 2023 17:58

mloubout reviewed Aug 9, 2023

View reviewed changes

EdCaunt reviewed Aug 9, 2023

View reviewed changes

ZoeLeibowitz force-pushed the petsc_object branch from afdfd2f to 43f1f07 Compare November 1, 2023 11:29

ZoeLeibowitz added 6 commits November 1, 2023 11:36

Test.

2b2719a

add is_PetscObject

d93d7e2

add petscobj to __all__

bda2f71

replace abstractproperty with property and abstractmethod

0815722

start PetscObject class

f778c7b

attempt is_const functionality in PetscObject instead of AbstractObje…

1760332

…ct itself

ZoeLeibowitz added 25 commits November 1, 2023 12:22

remove second struct

bd8d92e

figured how to adjust iteration space if needed

34da299

running apart from PetscFunctionBegin

c73ea72

code acc runs

a84bc0e

fix so that x_m etc are not args to kernel

794e670

subdomain pass

b25e410

edits to subdomain iet pass version and python run

a39a4ad

remove conditionals when building rhs vector

c9cd114

change callback to utilise an xextended

53e6d36

switch to more variables rather than numbers

d049106

diff pass with more loops for bcs

feb0504

make petsc pass using DMDA

f61f594

add ccode file produced with dmda pass

b027908

turn off shifting

f594665

turn off shifting

01cfcbe

latest

656f769

remove ipythons

436e001

remove comment

fd8adc5

latest

57e81a3

remove ipython

dc47d0d

turn off petsc pass

71034fe

add functions to separate petsc pass

71f9561

turn off negative access bounds checks for petsc

5025616

remove global y vec

634e6cb

resort back

e9e5bb2

ZoeLeibowitz force-pushed the petsc_object branch from 43f1f07 to e9e5bb2 Compare November 1, 2023 12:30

git checkout origin/master misc file

d6f2182

ZoeLeibowitz added testing Playing around. Don't review. and removed enhancement New feature or request labels Dec 19, 2023

ZoeLeibowitz closed this Mar 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start PetscObject #1

Start PetscObject #1

ZoeLeibowitz commented Jun 20, 2023

ZoeLeibowitz commented Jun 23, 2023

mloubout commented Jun 23, 2023 •

edited

Loading

FabioLuporini Jun 29, 2023

ZoeLeibowitz Jun 29, 2023

FabioLuporini Jun 29, 2023

FabioLuporini Jun 29, 2023

FabioLuporini Jun 29, 2023

FabioLuporini Jun 29, 2023

EdCaunt Jun 30, 2023

EdCaunt Jun 30, 2023

EdCaunt Jun 30, 2023

EdCaunt Jun 30, 2023

ZoeLeibowitz commented Aug 3, 2023

FabioLuporini Aug 4, 2023

ZoeLeibowitz Aug 4, 2023

FabioLuporini Aug 4, 2023

FabioLuporini Aug 4, 2023

FabioLuporini commented Aug 4, 2023

mloubout Aug 4, 2023

ZoeLeibowitz Aug 8, 2023

FabioLuporini Aug 9, 2023

mloubout Aug 9, 2023

mloubout Aug 9, 2023

EdCaunt Aug 9, 2023

EdCaunt Aug 9, 2023

EdCaunt Aug 9, 2023

		@@ -1,14 +1,17 @@
		from ctypes import byref
		from ctypes import POINTER

		@@ -400,6 +401,37 @@ def _op(self):
		return '(%s)' % self.typ


		class FunctionPointer(sympy.Expr, Pickable):

Start PetscObject #1

Start PetscObject #1

Conversation

ZoeLeibowitz commented Jun 20, 2023

ZoeLeibowitz commented Jun 23, 2023

mloubout commented Jun 23, 2023 • edited Loading

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

ZoeLeibowitz commented Aug 3, 2023

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

FabioLuporini commented Aug 4, 2023

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

mloubout commented Jun 23, 2023 •

edited

Loading