FEniCS / ffcx

Next generation FEniCS Form Compiler for finite element forms
https://fenicsproject.org
Other
145 stars 38 forks source link

Fix #602 #604

Closed mscroggs closed 1 year ago

mscroggs commented 1 year ago

After this PR:

This is a slightly hacky fix for #602.

Happy for this to be closed if a better fix exists

edit: this feels even hackier now that it's been edited to make mypy pass

Close #602. Close https://github.com/FEniCS/dolfinx/issues/2757

garth-wells commented 1 year ago

What is real is the generated code supposed to do? Is it an artefact, or should it be creal?

chrisrichardson commented 1 year ago

It should not be creal as the input is double not _Complex double. It should not be anything. But ufl is asking for the real part of a number that is already real.

mscroggs commented 1 year ago

What is real is the generated code supposed to do? Is it an artefact, or should it be creal?

real is replaced with creal for double complex and crealf for float complex. It's currently not replaced for real types. This PR makes a replacement happen for these types

chrisrichardson commented 1 year ago

Maybe use "" instead of None, then it will just use parentheses, which have no effect. Also a bit hacky.

mscroggs commented 1 year ago

Maybe use "" instead of None, then it will just use parentheses, which have no effect. Also a bit hacky.

mypy also didn't like None. I went with "SPECIAL_CASE ()" and "SPECIAL_CASE 0 (as the latter will also allow dealing with imag(real type) = 0. But open to better suggestions

chrisrichardson commented 1 year ago

How about picking it up a bit sooner (before consulting the math table):

if c.args[0].dtype == L.DataType.REAL:
    if c.function in ("real", "conj"):
        return "(" + self.c_format(c.args[0]) + ")"
    elif c.function == "imag":
        return "0"
chrisrichardson commented 1 year ago

Or maybe we should handle it in lnodes.py before it gets here...

def _math_function(op, *args):
    name = op._ufl_handler_name_
    dtype = args[0].dtype
    if name in ("conj", "real") and dtype == DataType.REAL:
        assert len(args) == 1
        return args[0]
    if name == "imag" and dtype == DataType.REAL:
        assert len(args) == 1
        return LiteralFloat(0.0)
    return MathFunction(op._ufl_handler_name_, args)
chrisrichardson commented 1 year ago

I think this is the least "hacky", let's hope it passes all the tests...