Allow custom codegen class to be passed in to autowrap and codegen #12833
Conversation
sympy/utilities/codegen.py
Outdated
| "OCTAVE": OctaveCodeGen, | ||
| "RUST": RustCodeGen}.get(language.upper()) | ||
| if isinstance(language, type): | ||
| CodeGenClass = language |
There was a problem hiding this comment.
Should allow instances too. You sometimes need to instantiate the class to set settings.
There was a problem hiding this comment.
For the code gen class? Doesn't it just take the project name in __init__? I was originally hoping to be able to use an instance of a custom printer, but I didn't get that working because the code gen class instantiates the printer with different settings in a couple places (e.g. human, dereference, etc.).
There was a problem hiding this comment.
I think we'll have to live with this for now. Seems like the CCodeGen class would need to be reworked in the long run to accept a printer instance in its __init__ method for example.
|
I've cleaned up the interface to customizing the import sympy as sp
from sympy.printing.ccode import C99CodePrinter
from sympy.utilities.codegen import CCodeGen, codegen
from sympy.utilities.autowrap import autowrap
x = sp.symbols('x')
class CustomCCodePrinter(C99CodePrinter):
_kf = dict(C99CodePrinter._kf, exp='fastexp')
class CustomCCodeGen(CCodeGen):
printer_class = CustomCCodePrinter
preprocessor_extras = ['#include "fastexp.h"\n']
result = codegen(('expr', sp.exp(x)), CustomCCodeGen)
print(result[0][1])I'm still not seeing a way to use an instance of a custom code printer unless there is a way to update its settings like Also, having a |
|
This seems like a bad design for the codeprinter: |
|
I think this is a suitable solution to get this working. It just needs some tests to check if the preprocessor stuff was added in the source file and that the custom printer was properly set on the class. |
|
What's the plan to allow the user to set the custom class at the autowrap level? |
A minimally invasive solution might be writing a
It should already "just work": pass in the class as the edit: need to think more about checking |
|
I don't think we need to touch or even: Regarding |
|
OK, I pushed to this branch after talking with @moorepants This kind of "works": But it feels like a horrible hack (but if you have a function called It's getting late here so I'll hand over to you guys and will catch up tomorrow morning. (I feel like quoting Churchill here...) |
|
It's looking good! Kenny's gonna add the code to deal with manipulating the include statements. |
|
|
||
| printer = C99CodePrinter(settings={'user_functions': {'exp': 'fastexp'}}) | ||
| gen = C99CodeGen(printer=printer) | ||
| gen.preprocessor_statements.append('#include "fastexp.h"\n') |
There was a problem hiding this comment.
I think it'd be a bit nicer if the user didn't have to append a \n to these strings.
There was a problem hiding this comment.
Ok they're appended automatically now. It doesn't really hurt anything if the user wants to have a more complicated preprocessor statement.
moorepants
changed the title
ixjlyons
force-pushed
the
custom_codegen_class
branch
from
6e9af99
to
cfda64f
Compare
|
If this needs to go in the release, close this PR and reopen it against the 1.1 branch. |
|
Never mind you can change the base branch without creating a new PR. |
|
The test failures aren't caused by this PR. They are caused by the Should I resolve the merge conflicts myself, by the way? What is the appropriate way to do it? Rebase onto 1.1 or merge in 1.1? |
|
I usually just merge master on top and resolve merge conflicts in the merge commit. It would be nice if the tests could be changed so that they ignore comments in the code. Let me know if I can help. |
|
Ok, this should be ready to go as long as Travis is happy. |
|
Thanks for fixing the rust failure! |
|
A bunch of tests fail for me locally Also, can you go ahead and add yourself to the AUTHORS file (run |
|
Should be fixed now, though I'm now getting an error in an unrelated test locally in |
|
My tests pass in external and utilities: |
|
@ixjlyons What test is failing for your locally? Can you print the output? |
Hmm, can't seem to replicate now, but it was definitely |
|
it has a counter and will increment that number based on how many times you run autowrap. You probably just incremented the counter while testing, whereas the testing is expecting the code to be generated from the first run of autowrap. |
|
Ah, yep. It fails if I run the external autowrap tests first: |
|
Yes, reset the counter to |
// edited by skirpichev
This feature would allow users to create their own code printer for use in
autowrapandcodegen. Here's a working example.The
autowrapline is commented out because compilation fails without passing compiler flags to tell it where thefastexp.hheader is, but the generated code looks correct. Here's thecodegenoutput:Updates to the docstring of
autowrapandcodegenwould still be needed, as well as tests. Submitting now to get feedback on how the feature itself and potential side effects.