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.

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

C code generation line break issue #263

Closed
blumenta opened this issue Sep 23, 2015 · 12 comments · Fixed by #292
Closed

C code generation line break issue #263

blumenta opened this issue Sep 23, 2015 · 12 comments · Fixed by #292

Comments

@blumenta
Copy link

Hello, while using CythonMatrixGenerator and CMatrixGenerator I stumbled upon the following issue:
the generated C code appears to have some undesirable line breaks which render the code uncompilable. For example, this generated statement

double pydy_2382 = (1.0L/4.0L)pydy_1930pydy_203pydy_204pydy_2339pydy_2
95pydy_345*pydy_60;

yields the following gcc error:

error: expected ‘,’ or ‘;’ before numeric constant
95pydy_345pydy_60;

Of course there shouldn't be a line break in the middle of the variable name pydy_295. That happens when attempting to compile the C code on my own. When I try using the CythonMatrixGenerator.compile() method, the Exception('Failed to compile and import Cython module.') is raised, I guess because of the same issue.
For now I managed to correct the generated c code manually since there are only 2 occurrences of this error, but maybe this is an issue that could be easily fixed in pydy.

Thanks in advance,

Alejandro

@oliverlee
Copy link
Contributor

Thanks for reporting the issue. Can you give an example that will reproduce this error?

@oliverlee
Copy link
Contributor

This can probably be solved by setting TextWrapper.break_long_words to False but I haven't been able to reproduce the bug and am not sure why it occurs in the first place as the text in the less than the set textwidth:

pydy/pydy/utils.py

Lines 35 to 48 in df1e7fe

def wrap_and_indent(lines, indentation=4, width=79):
"""Returns a single string in which the lines have been indented and
wrapped into a block of text."""
# TODO : This will indent any lines that only contain a new line. Which
# may not be preferable.
new_lines = []
for line in lines:
if line != '\n':
wrapped = textwrap.wrap(line, width=width-indentation)
else:
wrapped = [line]
new_lines += wrapped
spacer = '\n' + ' ' * indentation
return ' ' * indentation + spacer.join(new_lines)

@moorepants
Copy link
Member

@blumenta Please provide an small example that demonstrates this bug, otherwise it is difficult for us to track it down.

@blumenta
Copy link
Author

blumenta commented Oct 9, 2015

Hi, sorry I have been trying to reproduce the bug in a small example but haven't been able to do so. It occurs in the code I'm working on, but I'm kind of in a bind because I am not currently allowed to distribute it. Maybe I could save the problematic expression in a text file directly and then have a script that loads it and runs the C code generator on it. I'll get back to you

@oliverlee
Copy link
Contributor

Thanks for the update. If you can save the expression to the text file that
would be great. If not, we can try writing a patch and having you test it
locally so let us know when you have a chance.
On Oct 9, 2015 11:20, "blumenta" notifications@github.com wrote:

Hi, sorry I have been trying to reproduce the bug in a small example but
haven't been able to do so. It occurs in the code I'm working on, but I'm
kind of in a bind because I am not currently allowed to distribute it.
Maybe I could save the problematic expression in a text file directly and
then have a script that loads it and runs the C code generator on it. I'll
get back to you


Reply to this email directly or view it on GitHub
#263 (comment).

@blumenta
Copy link
Author

Hi, you will find attached source code that reproduces the bug. I saved the
problematic expression in a file 'dMdx.txt', the script 'bug.py' loads this
expression and prints the generated C code to the files
'pydy_codegen_c.{c,h}', these can be directly compiled with gcc using the
Makefile also included in the tar ball. Compilation fails because of the
above mentioned issue.
Thank you for your patience, I hadn't had any time lately to deal with this
issue because of a lot of teaching duties.
I hope this will be helpful, kind regards,

Alejandro

2015-10-09 12:15 GMT+02:00 Oliver Lee notifications@github.com:

Thanks for the update. If you can save the expression to the text file that
would be great. If not, we can try writing a patch and having you test it
locally so let us know when you have a chance.
On Oct 9, 2015 11:20, "blumenta" notifications@github.com wrote:

Hi, sorry I have been trying to reproduce the bug in a small example but
haven't been able to do so. It occurs in the code I'm working on, but I'm
kind of in a bind because I am not currently allowed to distribute it.
Maybe I could save the problematic expression in a text file directly and
then have a script that loads it and runs the C code generator on it.
I'll
get back to you


Reply to this email directly or view it on GitHub
#263 (comment).


Reply to this email directly or view it on GitHub
#263 (comment).

@oliverlee
Copy link
Contributor

I think you forgot to attach the files (or maybe you can't do so by email).

@moorepants
Copy link
Member

@blumenta Please email to the pydy list (or our personal email address) or make a gist or something with the code.

@oliverlee
Copy link
Contributor

Text wrap doesn't know that it can insert newlines after multiplication or division operators. We see output like this:

    double pydy_1462 = (1.0L/8.0L)*input_1[1]*pydy_1317*pydy_25*pydy_29*pydy_41
    1*pydy_63*pydy_78*pydy_99;

and this:

    double pydy_1462 = (1.0L/8.0L)*input_1[1]*pydy_1317*pydy_25*pydy_29*pydy_41
    1*pydy_63*pydy_78*pydy_99;

oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 20, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
@oliverlee
Copy link
Contributor

Fixed here: #292

oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 20, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
@moorepants
Copy link
Member

@blumenta Can you verify that this patch fixes things for you?

oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 20, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 20, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
@blumenta
Copy link
Author

Hi, I just tested it and it works perfectly, nice work.

2015-10-20 19:35 GMT+02:00 Jason K. Moore notifications@github.com:

@blumenta https://github.com/blumenta Can you verify that this patch
fixes things for you?


Reply to this email directly or view it on GitHub
#263 (comment).

oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 21, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
oliverlee added a commit to oliverlee/pydy that referenced this issue Oct 21, 2015
Some generated expressions, particularly those that contain only '*' and
'/' operands, have no whitespace. In this case, the textwrap module may
break the line in the middle of a symbol or mathematical expression if
the line is long, resulting in code that cannot compile. This commit
adds whitespace before and after the '*', '/' operands so that textwrap
will have more options for inserting line breaks. The textwrap option
'break_long_words' is also set to True, allowing long lines in the event
that no whitespace is present.

This commit resolves pydy#263.
oliverlee added a commit that referenced this issue Oct 21, 2015
Fix codegen line break. This resolves #263.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants