request for review: fcode

3 views
Skip to first unread message

Toon Verstraelen

unread,
Jul 2, 2009, 6:27:19 AM7/2/09
to sy...@googlegroups.com
Hi All,

I have a rather workable implementation of fcode (the fortran counterpart of
ccode). You can grab it here: http://github.com/tovrstra/sympy/tree/issue1487

I think it is more or less OK to merge the patches? I tried to take into account
all the comments from issue 1487. Only non-top-level piecewise functions are not
supported yet. Appart from that, it generates nice Fortran code that one can
copy-paste and compile. fcode is also designed to play well with codegen in the
future. For the interested: more details can be found in
doc/src/modules/printing.txt

cheers,

Toon


--
Dr. ir. Toon Verstraelen
Center for Molecular Modeling
Ghent University
Proeftuinstraat 86
B9000 Gent
Belgium
Tel: +32 9 264 65 56
E-mail: Toon.Ver...@UGent.be
http://molmod.UGent.be/
http://molmod.UGent.be/code/

toon_verstraelen.vcf

Andy Ray Terrel

unread,
Jul 2, 2009, 11:12:29 PM7/2/09
to sy...@googlegroups.com
Wow this looks really nice, I'm glad to see others extending these
code generation features.

Some comments regarding the request for review, using the commit
194edc73e6743c666315821a0c2ead0eb93588a3 . Overall I am +1 for
pushing but I would like to see a few small changes, probably move
some of the comments into issues into themselves.

fcode.py
-----------

* I don't like the repeating of many of functions that ccode has as
well. It seems that we could have a low level code generator that is
general enough to abstract many things, such as printing the implicit
functions and constant values. For example print pow, some special
functions and constants. This is probably a whole new issue though not
something to stop the code from being pushed.


* The implicit_functions dict should only rename functions if
necessary. As it is this makes the code bloated.


* Can you do piecewise functions the same way as ccode does where you
have the _print_Piecewise function. Probably could go into
LowLevelCodePrinter as well.


* the line wrapping produces bad code (unreadable). There needs to be
work to take care of cases where operators will be split. For
example:

In [12]: e = [x**i for i in range(11)]

In [13]: print fcode(Add(*e))
-------> print(fcode(Add(*e)))
1 + x + x**2 + x**3 + x**4 + x**5 + x**6 + x**7 + x**8 + x**9 + x*
@ *10


* fcode function should move to something like the following signature
(expr, *args, **kws) and look up the keywords, it gets too many
optional keywords as is.


* Something that might be nice, instead of an exception being thrown
just have a list of functions that are in the expression but not
implemented in the core language. That way a code generator can look
at what needs to be implemented and prompt the user appropriately
instead of just halting.


* One test failed for me:

__________________________ sympy.printing.fcode.fcode __________________________
File "/Users/aterrel/workspace/sympy/sympy/printing/fcode.py", line
252, in sympy.printing.fcode.fcode
Failed example:
fcode((2*tau)**Rational(7,2))
Expected:
' 8*2**(1.0/2.0)*tau**(7.0/2.0)'
Got:
' 8*sqrt(2)*tau**(7.0/2.0)'


The tests and docs look good. We should probably take some time and
make ccode this good as well.


-- Andy

Tim Lahey

unread,
Jul 2, 2009, 11:19:01 PM7/2/09
to sy...@googlegroups.com

On Jul 2, 2009, at 11:12 PM, Andy Ray Terrel wrote:

>
> Wow this looks really nice, I'm glad to see others extending these
> code generation features.
>
> Some comments regarding the request for review, using the commit
> 194edc73e6743c666315821a0c2ead0eb93588a3 . Overall I am +1 for
> pushing but I would like to see a few small changes, probably move
> some of the comments into issues into themselves.
>
> fcode.py
> -----------
>
> * I don't like the repeating of many of functions that ccode has as
> well. It seems that we could have a low level code generator that is
> general enough to abstract many things, such as printing the implicit
> functions and constant values. For example print pow, some special
> functions and constants. This is probably a whole new issue though not
> something to stop the code from being pushed.

Maple's code generator converts to an intermediate format and then
is converted to the final form. That may be an idea if multiple
languages
are going to be supported. That way, some common procedures can be
done and then the language specific forms/optimizations can be done
after.

Like Andy says, it's not a reason to stop the code, but it's a
possibility
since things like MATLAB/Octave or Scilab could be other possible
outputs.

Cheers,

Tim.

---
Tim Lahey
PhD Candidate, Systems Design Engineering
University of Waterloo
http://www.linkedin.com/in/timlahey

Toon Verstraelen

unread,
Jul 3, 2009, 2:58:08 AM7/3/09
to sy...@googlegroups.com
Tim Lahey wrote:
> On Jul 2, 2009, at 11:12 PM, Andy Ray Terrel wrote:
>> * I don't like the repeating of many of functions that ccode has as well.
>> It seems that we could have a low level code generator that is general
>> enough to abstract many things, such as printing the implicit functions and
>> constant values. For example print pow, some special functions and
>> constants. This is probably a whole new issue though not something to stop
>> the code from being pushed.

I'm all in favor of a LowLevelCodePrinter. I just did fcode first to have an
empty playground. I would also suggest a BasicStrPrinter between Printer and
StrPrinter, which does no more than +, -, *, /, ** and brackets. Such a
BasicStrPrinter would also serve as a good base class the LowLevelCodePrinter.

> Maple's code generator converts to an intermediate format and then is
> converted to the final form. That may be an idea if multiple languages are
> going to be supported. That way, some common procedures can be done and then
> the language specific forms/optimizations can be done after.

With the intermediate format, do you mean a string format, or a full blown data
structure? The latter approach is followed in sympy/utilities/codegen.py, but
I'm not sure if it is also good for the individual expressions? Such a data
structure would overlap a lot with the data structure for sympy expressions.

> Like Andy says, it's not a reason to stop the code, but it's a possibility
> since things like MATLAB/Octave or Scilab could be other possible outputs.

Thanks for the suggestion. It's on the idea list.

toon_verstraelen.vcf

Toon Verstraelen

unread,
Jul 3, 2009, 3:45:24 AM7/3/09
to sy...@googlegroups.com
Andy Ray Terrel wrote:
> * The implicit_functions dict should only rename functions if
> necessary. As it is this makes the code bloated.

It is also used to check if a function is an implicit function. We can split up
the dictionary into a set and a dictionary in this style:

implicit_functions = set([sin, cos, tan, asin, acos, atan, atan2, sinh, cosh,
tanh, sqrt, log, exp, abs, sign])
# The following are also considered to be implicit functions, but with a
# different fortran name:
translate_functions = {conjugate: "conjg"}

> * Can you do piecewise functions the same way as ccode does where you
> have the _print_Piecewise function. Probably could go into
> LowLevelCodePrinter as well.

This is indeed a fly in our soup. The approach in ccode is not workable when
there are non-top-level piecewise functions.

>>> ccode(Piecewise((x, x<1),(1/x,True))**2)
'pow(if (x < 1) {\nx\n}\nelse {\n1.0/x\n},2)'

One can easily see that a 'buried' Piecewise can only be handled by
_print_Piecewise if the target language supports a conditional operator. Fortran
does not have one, C does but is not used in ccode. Other solutions are not
compatible with the Printer approach: introducing temporary variables, or only
allowing a top-level Piecewise.

If we want to translate buried Piecewise functions, we will have to do this
outside the printer. The above expression should be split in multiple
expressions before it can be fed to a printer:

>>> print fcode(Piecewise((x, x<1),(1/x,True)), assign_to="x1")
if (x < 1) then
x1 = x
else
x1 = 1.0/x
end if
>>> print fcode(x1**2, assign_to="x2")
x2 = x1**2

Any suggestions for a more elegant approach are highly welcome. :) The splitting
is very similar to common subexpression elimination, which I would like to
include in codegen.py. Therefore, it might be a good idea to treat both problems
in codegen.py with a generalized cse algorithm.

> * the line wrapping produces bad code (unreadable). There needs to be
> work to take care of cases where operators will be split. For
> example:
>
> In [12]: e = [x**i for i in range(11)]
>
> In [13]: print fcode(Add(*e))
> -------> print(fcode(Add(*e)))
> 1 + x + x**2 + x**3 + x**4 + x**5 + x**6 + x**7 + x**8 + x**9 + x*
> @ *10

Will be fixed. Thanks for the nice example.

> * fcode function should move to something like the following signature
> (expr, *args, **kws) and look up the keywords, it gets too many
> optional keywords as is.

ok

> * Something that might be nice, instead of an exception being thrown
> just have a list of functions that are in the expression but not
> implemented in the core language. That way a code generator can look
> at what needs to be implemented and prompt the user appropriately
> instead of just halting.

That is indeed a better approach. It is a user friendly alternative to the
exceptions. Thanks for the suggestion.

> * One test failed for me:
>
> __________________________ sympy.printing.fcode.fcode __________________________
> File "/Users/aterrel/workspace/sympy/sympy/printing/fcode.py", line
> 252, in sympy.printing.fcode.fcode
> Failed example:
> fcode((2*tau)**Rational(7,2))
> Expected:
> ' 8*2**(1.0/2.0)*tau**(7.0/2.0)'
> Got:
> ' 8*sqrt(2)*tau**(7.0/2.0)'

oops.

> The tests and docs look good. We should probably take some time and
> make ccode this good as well.

As soon as fcode is 'in', ccode will be brought to the same level, and both will
be based on a low-level code printer. I'll first introduce all your suggestions.
Thanks for reviewing.

cheers,

Toon

toon_verstraelen.vcf

Andy Ray Terrel

unread,
Jul 3, 2009, 11:07:59 AM7/3/09
to sy...@googlegroups.com
Yeah piecewise functions are still only about 50% implemented. I
wanted to make a piecewise_fold function that could be called to fold
any outer arguments into the piecewise exprs, but got stuck a while
back on the assumption system. Perhaps I should get that working
again.

http://github.com/aterrel/sympy/commits/piecewise

and issue 353

As far as the intermediate rep for other languages, I think just using
the arg tree already given by the symbols is best. I've created an
issue for the LowLevelCodePrinter (issue 1514)

-- Andy

Toon Verstraelen

unread,
Jul 3, 2009, 11:28:51 PM7/3/09
to sy...@googlegroups.com
Andy Ray Terrel wrote:
> Yeah piecewise functions are still only about 50% implemented. I
> wanted to make a piecewise_fold function that could be called to fold
> any outer arguments into the piecewise exprs, but got stuck a while
> back on the assumption system. Perhaps I should get that working
> again.
>
> http://github.com/aterrel/sympy/commits/piecewise
>
> and issue 353

Fold is the way to go. It's the most elegant idea so far and it seems to
be required by several other parts of sympy to handle piecewise
functions properly. Issue 353 is blocked on 1047, so we can give it a
little rest for now.

> As far as the intermediate rep for other languages, I think just using
> the arg tree already given by the symbols is best. I've created an
> issue for the LowLevelCodePrinter (issue 1514)

thx

Toon Verstraelen

unread,
Jul 6, 2009, 6:34:44 AM7/6/09
to sy...@googlegroups.com
Andy Ray Terrel wrote:
> * fcode function should move to something like the following signature
> (expr, *args, **kws) and look up the keywords, it gets too many
> optional keywords as is.

Are there convenient ways to check the validity of *args and **kwargs? The code
to process *arg and **kwargs becomes daunting when done properly. The
conventional way is actually really compact. I can not find something that can
replace

def fcode(expr, assign_to=None, precision=15, user_functions={}, human=True):

without introducing ten times the amount of code. If there is no elegant way to
do this, I don't see the purpose of *args and **kwargs.

cheers,
Toon

Andy Ray Terrel

unread,
Jul 6, 2009, 12:28:29 PM7/6/09
to sy...@googlegroups.com
Yeah I hear you. As a general rule I like to keep less than 5
arguments to a function for readability and maintainability, but the
flip side is a dispatch function with a lot of ( if !kwargs.has("foo")
). These code dispatchers seem like they will rapidly grow in number
of args so I wanted to nip that bud early, but if you feel it is too
much just drop it.

-- Andy

>
> cheers,
> Toon
>
> >
>
Reply all
Reply to author
Forward
0 new messages