improved print_constant #1120

bauom · 2022-05-30T17:25:04Z

Print unknown constants using their values. This fixes #1047 but may be reverted in #1811 to prefer declaring constants with names. However this PR also adds support for NaN in C and an associated test. It also removes the duplicate _print_Constant function.

pyccel/codegen/printing/ccode.py

EmilyBourne · 2022-05-31T11:16:15Z

Is this PR ready for its initial review or is it still in draft status? Both are indicated at the moment

bauom · 2022-05-31T13:34:44Z

Is this PR ready for its initial review or is it still in draft status? Both are indicated at the moment

i was explaining to the new interns something and i forgot it my bad.

EmilyBourne · 2022-11-08T11:01:36Z

@bauom what is the status of this PR?

bauom · 2022-11-09T09:50:59Z

@bauom what is the status of this PR?

sorry i have totally forgotten about this PR i will fix it ASAP.

bauom · 2022-12-21T18:10:18Z

@EmilyBourne
i don't know if the issue below is a know issue.
the code below gives to different errors when using pyccel vs epyccel

def ret_pi():
    from math import pi
    return pi

pyccel error:

|fatal [syntax]: epyccel.py [29,11]| Pyccel has encountered syntax that has not been implemented yet. Please create an issue at https://github.com/pyccel/pyccel/issues and provide a small example of your problem. Do not forget to specify your target language (.join)

i used this code to run it with epyccel:

from numpy import pi
from pyccel.epyccel import epyccel
import numpy as np

def ret_pi():
    from math import pi
    return pi


def test_pi(language):
    f = ret_pi
    pf = epyccel(f, language=language)
    assert np.equal(f(), pf() )

test_pi('c')

epyccel error:

ERROR at shared library generation stage
Traceback (most recent call last):
  File "/home/bauom/dev/sandbox/test.py", line 15, in <module>
    test_pi('c')
  File "/home/bauom/dev/sandbox/test.py", line 12, in test_pi
    pf = epyccel(f, language=language)
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 333, in epyccel
    mod, fun = epyccel_seq( python_function_or_module, **kwargs )
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 168, in epyccel_seq
    execute_pyccel(pymod_filename,
  File "/home/bauom/dev/pyccel/pyccel/codegen/pipeline.py", line 386, in execute_pyccel
    generated_filepath = create_shared_library(codegen,
  File "/home/bauom/dev/pyccel/pyccel/codegen/python_wrapper.py", line 135, in create_shared_library
    wrapper_compiler.compile_module(wrapper_compile_obj,
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 305, in compile_module
    self.run_command(cmd, verbose)
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 429, in run_command
    raise RuntimeError(err_msg)
RuntimeError: Failed to build module
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c: In function ‘ret_pi_wrapper’:
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:30:10: error: lvalue required as left operand of assignment
   30 |     M_PI = ret_pi();
      |          ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:31:33: error: lvalue required as unary ‘&’ operand
   31 |     pi_tmp = Double_to_PyDouble(&M_PI);
      |                                 ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:20:12: warning: unused variable ‘pi’ [-Wunused-variable]
   20 |     double pi;
      |            ^~

EmilyBourne · 2022-12-21T19:19:57Z

@EmilyBourne
i don't know if the issue below is a know issue.
the code below gives to different errors when using pyccel vs epyccel

def ret_pi():
    from math import pi
    return pi

pyccel error:

|fatal [syntax]: epyccel.py [29,11]| Pyccel has encountered syntax that has not been implemented yet. Please create an issue at https://github.com/pyccel/pyccel/issues and provide a small example of your problem. Do not forget to specify your target language (.join)

i used this code to run it with epyccel:

from numpy import pi
from pyccel.epyccel import epyccel
import numpy as np

def ret_pi():
    from math import pi
    return pi


def test_pi(language):
    f = ret_pi
    pf = epyccel(f, language=language)
    assert np.equal(f(), pf() )

test_pi('c')

epyccel error:

ERROR at shared library generation stage
Traceback (most recent call last):
  File "/home/bauom/dev/sandbox/test.py", line 15, in <module>
    test_pi('c')
  File "/home/bauom/dev/sandbox/test.py", line 12, in test_pi
    pf = epyccel(f, language=language)
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 333, in epyccel
    mod, fun = epyccel_seq( python_function_or_module, **kwargs )
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 168, in epyccel_seq
    execute_pyccel(pymod_filename,
  File "/home/bauom/dev/pyccel/pyccel/codegen/pipeline.py", line 386, in execute_pyccel
    generated_filepath = create_shared_library(codegen,
  File "/home/bauom/dev/pyccel/pyccel/codegen/python_wrapper.py", line 135, in create_shared_library
    wrapper_compiler.compile_module(wrapper_compile_obj,
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 305, in compile_module
    self.run_command(cmd, verbose)
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 429, in run_command
    raise RuntimeError(err_msg)
RuntimeError: Failed to build module
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c: In function ‘ret_pi_wrapper’:
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:30:10: error: lvalue required as left operand of assignment
   30 |     M_PI = ret_pi();
      |          ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:31:33: error: lvalue required as unary ‘&’ operand
   31 |     pi_tmp = Double_to_PyDouble(&M_PI);
      |                                 ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:20:12: warning: unused variable ‘pi’ [-Wunused-variable]
   20 |     double pi;
      |            ^~

There is no difference between pyccel and epyccel, you are trying to call pyccel on the file containing the epyccel command, therefore you are trying to pyccelise epyccel.py. If you just translate the function then you shouldn't have a problem.

I'm not sure about the second half though, it looks like you named a variable pi, but I don't see that in your code

bauom · 2022-12-29T10:08:55Z

@EmilyBourne
i don't know if the issue below is a know issue.
the code below gives to different errors when using pyccel vs epyccel

def ret_pi():
    from math import pi
    return pi

pyccel error:

|fatal [syntax]: epyccel.py [29,11]| Pyccel has encountered syntax that has not been implemented yet. Please create an issue at https://github.com/pyccel/pyccel/issues and provide a small example of your problem. Do not forget to specify your target language (.join)

i used this code to run it with epyccel:

from numpy import pi
from pyccel.epyccel import epyccel
import numpy as np

def ret_pi():
    from math import pi
    return pi


def test_pi(language):
    f = ret_pi
    pf = epyccel(f, language=language)
    assert np.equal(f(), pf() )

test_pi('c')

epyccel error:

ERROR at shared library generation stage
Traceback (most recent call last):
  File "/home/bauom/dev/sandbox/test.py", line 15, in <module>
    test_pi('c')
  File "/home/bauom/dev/sandbox/test.py", line 12, in test_pi
    pf = epyccel(f, language=language)
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 333, in epyccel
    mod, fun = epyccel_seq( python_function_or_module, **kwargs )
  File "/home/bauom/dev/pyccel/pyccel/epyccel.py", line 168, in epyccel_seq
    execute_pyccel(pymod_filename,
  File "/home/bauom/dev/pyccel/pyccel/codegen/pipeline.py", line 386, in execute_pyccel
    generated_filepath = create_shared_library(codegen,
  File "/home/bauom/dev/pyccel/pyccel/codegen/python_wrapper.py", line 135, in create_shared_library
    wrapper_compiler.compile_module(wrapper_compile_obj,
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 305, in compile_module
    self.run_command(cmd, verbose)
  File "/home/bauom/dev/pyccel/pyccel/codegen/compiling/compilers.py", line 429, in run_command
    raise RuntimeError(err_msg)
RuntimeError: Failed to build module
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c: In function ‘ret_pi_wrapper’:
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:30:10: error: lvalue required as left operand of assignment
   30 |     M_PI = ret_pi();
      |          ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:31:33: error: lvalue required as unary ‘&’ operand
   31 |     pi_tmp = Double_to_PyDouble(&M_PI);
      |                                 ^
/home/bauom/dev/sandbox/__epyccel__/__pyccel__/mod_l2hcaikn18un_l2hcaikn18un_wrapper.c:20:12: warning: unused variable ‘pi’ [-Wunused-variable]
   20 |     double pi;
      |            ^~

There is no difference between pyccel and epyccel, you are trying to call pyccel on the file containing the epyccel command, therefore you are trying to pyccelise epyccel.py. If you just translate the function then you shouldn't have a problem.

I'm not sure about the second half though, it looks like you named a variable pi, but I don't see that in your code

those are two file different
For pyccel i am just pyccelising the function ret_pi() the file only contains

def ret_pi():
    from math import pi
    return pi

I think there is a problem when we try to return a constant value that is an import but i am not sure if these are known issues.

EmilyBourne · 2022-12-29T10:11:23Z

This is not a known problem. It is really weird!! We definitely need an issue for this

bauom · 2022-12-29T10:15:26Z

This is not a known problem. It is really weird!! We definitely need an issue for this

I will create an issue for this not sure if these are 2 issues different or the same problem that cause different effects.

EmilyBourne · 2022-12-29T10:18:24Z

This is not a known problem. It is really weird!! We definitely need an issue for this

I will create an issue for this not sure if these are 2 issues different or the same problem that cause different effects.

I'm looking into it. It looks like the error is caused by the results of the FunctionDef at the syntactic stage

pyccel/codegen/printing/ccode.py

pyccel-bot · 2023-07-28T11:18:30Z

Hello! Welcome to Pyccel! Thank you very much for your contribution ❤️.

I am the GitHub bot. I will help guide you through the different stages necessary to validate a review in Pyccel. If you haven't yet seen our developer docs make sure you check them out here. Amongst other things they describe the review process that we have just started. You can also get in touch with our other developers on our Pyccel Discord Server.

To begin with I will give you a short checklist to make sure your pull request is complete. Please tick items off when you have completed them or determined that they are not necessary for this pull request. If you want me to run any specific tests to check out corner cases that you can't easily check on your computer, you can request this using the command /bot run X. Use the command /bot show tests to see a full list of the tests I can run. Once you have finished preparing your pull request and are ready to request reviews just take your PR out of draft, or let me know with the command /bot mark as ready. I will then run the full suite of tests to check that everything is as neat as you think before asking other contributors for reviews. You can get a full list of commands that I understand using /bot commands.

Here is your checklist:

Update AUTHORS file
Write a clear PR description
Add tests to check your code works as expected
Update documentation if necessary
Update Changelog
Ensure any relevant issues are linked
Ensure new tests are passing

EmilyBourne · 2023-12-14T14:03:43Z

@bauom Yearly reminder about the existence of this PR ;)

EmilyBourne · 2024-03-29T14:28:15Z

@bauom This is now blocking #1807 . Do you want me to finish it?

pyccel-bot

Good job ! Your PR is using all the code it added/changed.

pyccel-bot · 2024-05-03T05:10:24Z

Hey @pyccel/pyccel-dev ! @bauom has just created this great new pull request! Check it out and let me know what you think!

smazouz42

Approved. Looks good to merge

pyccel-bot

Good job ! Your PR is using all the code it added/changed.

pyccel-bot · 2024-05-12T16:58:10Z

Hey @yguclu, @EmilyBourne, this PR is looking pretty good. @bauom and @smazouz42 think it is ready to merge. Could you add your expertise to confirm that this follows all the coding conventions and fits in Pyccel's future plans? Thanks 😄

EmilyBourne

I am ok with this PR as is. Some of the work will be superceded in #1811 so it is not critical if it is not perfect.
However I finished cleaning it up so it is still missing a senior review.

yguclu

Looks good to me. I just have one question.

pyccel/codegen/printing/fcode.py

pyccel-bot · 2024-05-13T14:19:45Z

@bauom, @yguclu has a few questions/comments about your code. Can you go through and see if you agree with them. If not go ahead and explain why. Once you've adressed all the comments let me know with /bot mark as ready and we will see if we can get approval.

pyccel-bot

Good job ! Your PR is using all the code it added/changed.

Print unknown constants using their values. This fixes pyccel#1047 but may be reverted in pyccel#1811 to prefer declaring constants with names. However this PR also adds support for `NaN` in C and an associated test. It also removes the duplicate `_print_Constant` function. --------- Co-authored-by: EmilyBourne <louise.bourne@gmail.com>

added handling of float constant in print_constant

cd8ada4

bauom added the needs_initial_review label May 30, 2022

bauom requested a review from ohachim May 30, 2022 17:33

EmilyBourne reviewed May 31, 2022

View reviewed changes

pyccel/codegen/printing/ccode.py Show resolved Hide resolved

bauom removed the needs_initial_review label May 31, 2022

bauom and others added 2 commits November 8, 2022 23:55

Merge branch 'master' into devel-1047

1d33353

using C constants

152c85b

Merge branch 'master' into devel-1047

cdf8c9b

removed duplicated const printer

b27fcab

EmilyBourne reviewed Dec 29, 2022

View reviewed changes

pyccel/codegen/printing/ccode.py Outdated Show resolved Hide resolved

yguclu assigned bauom Mar 3, 2023

EmilyBourne added 3 commits April 13, 2023 13:19

Merge branch 'master' into devel-1047

8d4004e

Merge branch 'devel' into devel-1047

5c0aee3

Merge branch 'devel' into devel-1047

ca7d957

EmilyBourne closed this Jul 28, 2023

EmilyBourne reopened this Jul 28, 2023

Merge branch 'devel' into devel-1047

9dcc77d

pyccel-bot bot approved these changes May 2, 2024

View reviewed changes

pyccel-bot bot added the needs_initial_review label May 3, 2024

pyccel-bot bot requested a review from a team May 3, 2024 05:10

EmilyBourne unassigned bauom May 3, 2024

smazouz42 approved these changes May 9, 2024

View reviewed changes

Merge branch 'devel' into devel-1047

53c4f24

EmilyBourne marked this pull request as draft May 12, 2024 16:38

EmilyBourne marked this pull request as ready for review May 12, 2024 16:38

pyccel-bot bot approved these changes May 12, 2024

View reviewed changes

pyccel-bot bot removed the needs_initial_review label May 12, 2024

pyccel-bot bot added the Ready_for_review Received at least one approval. Requires review from senior developer label May 12, 2024

pyccel-bot bot requested review from EmilyBourne and yguclu May 12, 2024 16:58

EmilyBourne approved these changes May 12, 2024

View reviewed changes

pyccel-bot bot added Ready_to_merge Approved by senior developer. Ready for final approval and merge and removed Ready_for_review Received at least one approval. Requires review from senior developer labels May 12, 2024

yguclu requested changes May 13, 2024

View reviewed changes

pyccel/codegen/printing/fcode.py Show resolved Hide resolved

github-actions bot marked this pull request as draft May 13, 2024 14:19

pyccel-bot bot removed the Ready_to_merge Approved by senior developer. Ready for final approval and merge label May 13, 2024

Merge branch 'devel' into devel-1047

4197662

yguclu approved these changes May 13, 2024

View reviewed changes

EmilyBourne marked this pull request as ready for review May 13, 2024 15:07

pyccel-bot bot approved these changes May 13, 2024

View reviewed changes

pyccel-bot bot added the Ready_to_merge Approved by senior developer. Ready for final approval and merge label May 13, 2024

EmilyBourne merged commit d64be7f into devel May 14, 2024
14 checks passed

EmilyBourne deleted the devel-1047 branch May 14, 2024 06:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improved print_constant #1120

improved print_constant #1120

bauom commented May 30, 2022 •

edited by EmilyBourne

EmilyBourne commented May 31, 2022

bauom commented May 31, 2022

EmilyBourne commented Nov 8, 2022

bauom commented Nov 9, 2022

bauom commented Dec 21, 2022

EmilyBourne commented Dec 21, 2022

bauom commented Dec 29, 2022

EmilyBourne commented Dec 29, 2022

bauom commented Dec 29, 2022 •

edited

EmilyBourne commented Dec 29, 2022

pyccel-bot bot commented Jul 28, 2023 •

edited by EmilyBourne

EmilyBourne commented Dec 14, 2023

EmilyBourne commented Mar 29, 2024

pyccel-bot bot left a comment

pyccel-bot bot commented May 3, 2024

smazouz42 left a comment

pyccel-bot bot left a comment

pyccel-bot bot commented May 12, 2024

EmilyBourne left a comment •

edited

yguclu left a comment

pyccel-bot bot commented May 13, 2024

pyccel-bot bot left a comment

improved print_constant #1120

improved print_constant #1120

Conversation

bauom commented May 30, 2022 • edited by EmilyBourne

EmilyBourne commented May 31, 2022

bauom commented May 31, 2022

EmilyBourne commented Nov 8, 2022

bauom commented Nov 9, 2022

bauom commented Dec 21, 2022

EmilyBourne commented Dec 21, 2022

bauom commented Dec 29, 2022

EmilyBourne commented Dec 29, 2022

bauom commented Dec 29, 2022 • edited

EmilyBourne commented Dec 29, 2022

pyccel-bot bot commented Jul 28, 2023 • edited by EmilyBourne

EmilyBourne commented Dec 14, 2023

EmilyBourne commented Mar 29, 2024

pyccel-bot bot left a comment

Choose a reason for hiding this comment

pyccel-bot bot commented May 3, 2024

smazouz42 left a comment

Choose a reason for hiding this comment

pyccel-bot bot left a comment

Choose a reason for hiding this comment

pyccel-bot bot commented May 12, 2024

EmilyBourne left a comment • edited

Choose a reason for hiding this comment

yguclu left a comment

Choose a reason for hiding this comment

pyccel-bot bot commented May 13, 2024

pyccel-bot bot left a comment

Choose a reason for hiding this comment

bauom commented May 30, 2022 •

edited by EmilyBourne

bauom commented Dec 29, 2022 •

edited

pyccel-bot bot commented Jul 28, 2023 •

edited by EmilyBourne

EmilyBourne left a comment •

edited