Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler: Fix dtype of FFP and edit sympy_dtype #2513

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

compiler: Fix dtype of FFP and edit sympy_dtype #2513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions devito/symbolics/extended_sympy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ def __str__(self):
def field(self):
return self.call

@property
def dtype(self):
return self.field.dtype

__repr__ = __str__


Expand Down
9 changes: 4 additions & 5 deletions devito/symbolics/inspection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,8 @@ def sympy_dtype(expr, base=None):
Infer the dtype of the expression.
"""
dtypes = {base} - {None}
for i in expr.free_symbols:
try:
dtypes.add(i.dtype)
except AttributeError:
pass
for i in expr.args:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work for nested expression, why does the previous one not work?

Copy link
Contributor Author

@ZoeLeibowitz ZoeLeibowitz Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It passes all tests I believe (ofc without the ascent import issue).. so I'm not sure what kind of expression you are referring to? Will the nested expression not have a dtype attribute? Maybe I need singledispatch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching through expr.free_symbols for types like FieldFromPointer is problematic because it examines the dtype of the pointer itself rather than the dtype of the field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then you may need to use singledispatch to implement sympy_dtype (which, btw, I'd rename into something else as "dtype" isn't even a SymPy thing; like, extract_dtype or something like that)

An alternative would be to turn it into a recursive function (check expr; if not enough, check its arguments and for each argument call the function recursively), but it might not be enough for special objects like FieldFromPointer

dtype = getattr(i, 'dtype', None)
if dtype:
dtypes.add(dtype)
return infer_dtype(dtypes)
17 changes: 14 additions & 3 deletions tests/test_symbolics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
CallFromPointer, Cast, DefFunction, FieldFromPointer,
INT, FieldFromComposite, IntDiv, Namespace, Rvalue,
ReservedWord, ListInitializer, ccode, uxreplace,
retrieve_derivatives)
retrieve_derivatives, sympy_dtype)
from devito.tools import as_tuple
from devito.types import (Array, Bundle, FIndexed, LocalObject, Object,
Symbol as dSymbol)
Symbol as dSymbol, CompositeObject)
from devito.types.basic import AbstractSymbol


Expand Down Expand Up @@ -248,6 +248,17 @@ def test_field_from_pointer():
# Free symbols
assert ffp1.free_symbols == {s}

# Test dtype
f = dSymbol('f')
pfields = [(f._C_name, f._C_ctype)]
struct = CompositeObject('s1', 'myStruct', pfields)
ffp4 = FieldFromPointer(f, struct)
assert str(ffp4) == 's1->f'
assert ffp4.dtype == f.dtype
expr = 1/ffp4
dtype = sympy_dtype(expr)
assert dtype == f.dtype


def test_field_from_composite():
s = Symbol('s')
Expand Down Expand Up @@ -292,7 +303,7 @@ def test_extended_sympy_arithmetic():
# noncommutative
o = Object(name='o', dtype=c_void_p)
bar = FieldFromPointer('bar', o)
assert ccode(-1 + bar) == '-1 + o->bar'
assert ccode(-1 + bar) == 'o->bar - 1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the order change?



def test_integer_abs():
Expand Down
Loading