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

pyccel acceleration does not seem to work in C #431

Open
campospinto opened this issue Sep 12, 2024 · 5 comments
Open

pyccel acceleration does not seem to work in C #431

campospinto opened this issue Sep 12, 2024 · 5 comments
Labels
bug Something isn't working codegen Automatic code generation

Comments

@campospinto
Copy link
Collaborator

The pyccelization of some kernels does not seem to work in C.

For instance, with the current version of devel, the command
python psydac_accelerate.py --language c yields the following errors:

  Pyccelize file: /Users/campos/Work/codes/pyccel/psydac/psydac/core/bsplines_kernels.py

ERROR at code generation stage
pyccel:
 |fatal [codegen]: bsplines_kernels.py [326,27]| _print_NumpyMatmul is not yet implemented for language : C
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. (<pyccel.ast.numpyext.NumpyMatmul object at 0x11e846dc0>)

and

  Pyccelize file: /Users/campos/Work/codes/pyccel/psydac/psydac/core/field_evaluation_kernels.py

ERROR at code generation stage
pyccel:
 |fatal [codegen]: field_evaluation_kernels.py [2326,56]| _print_PythonList is not yet implemented for language : C
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. ((x_x1, x_x2, x_x3))

I am running this on Mac OS but the error seems also present on other machines

@campospinto campospinto added bug Something isn't working codegen Automatic code generation labels Sep 12, 2024
@campospinto
Copy link
Collaborator Author

Some of these errors seem to be addressed in the Struphy psydac fork https://github.com/spossann/stefan-psydac, see for instance the commits

Enable compilation with language C

and

Comment jacobian matrix eval kernels; lists not supported yet for c-c

@max-models
Copy link
Contributor

I replaced the python lists in field_evaluation_kernels.py.

I can successfully use pyccel psydac/core/field_evaluation_kernels.py --language=c on my branch now (stefan-psydac had simply commented out all the kernels).

For bsplines_kernels.py, we need to replace np.matmul, np.sum, sum, np.min, and np.max. This is done in stefan-psydac in psydac/core/arrays.py which does not exist in psydac devel.

@yguclu
Copy link
Member

yguclu commented Sep 18, 2024

@campospinto @spossann @maxlin-ipp: Why do you want to use the C backend instead of the Fortran one?

As of Pyccel 1.11.2, the Fortran backend is much better than the C one, both in terms of coverage of the Python language, and of performance of the accelerated functions. Therefore I still recommend Pyccel users to choose backend='fortran' for now, unless there is a very good reason not to do so.

In the next version of Pyccel (1.11.3 or 2.0.0) the C backend will certainly be much more competitive. It should be able to translate the Psydac kernels out of the box. (cc: @EmilyBourne)

@EmilyBourne
Copy link
Member

I replaced the python lists in field_evaluation_kernels.py.

I can successfully use pyccel psydac/core/field_evaluation_kernels.py --language=c on my branch now (stefan-psydac had simply commented out all the kernels).

For bsplines_kernels.py, we need to replace np.matmul, np.sum, sum, np.min, and np.max. This is done in stefan-psydac in psydac/core/arrays.py which does not exist in psydac devel.

I don't think np.sum or sum are necessary any more. They have been added to Pyccel since.

As I mentioned to Stefan at the time a more helpful solution would be to help add support for this in Pyccel to benefit the community generally (if this had been done it would have worked out of the box for you).

I don't currently have min/max/matmul on the roadmap for v2.0.0 but they can be added to make this a priority.

As mentioned by Yaman we are currently improving the implementation of arrays in C. None of the improvements can therefore be looked at until pyccel/pyccel#1969 is completed. Hopefully some of these things may be easier to implement once that is done.

campospinto added a commit that referenced this issue Sep 18, 2024
current README mentions possibility of selecting C as pyccel backend,
but not always possible, see issue #431

---------

Co-authored-by: Yaman Güçlü <[email protected]>
yguclu added a commit that referenced this issue Sep 19, 2024
Clarify in the `README.md` file that, although the C backend may be
selected for accelerating the kernel files with Pyccel, this is not
fully working yet. Hence the Fortran backend (which is the default) is
the only one available. A future version of Pyccel will certainly
provide a C backend as capable as the Fortran one. See issue #431.

Co-authored-by: Martin Campos Pinto <[email protected]>
@EmilyBourne
Copy link
Member

We have just noticed that np.min(x), np.amin(x), and x.min() are supported for NumPy arrays but min(x) does not currently work. It may be that you can just ensure that you are calling the NumPy function in your code

max-models pushed a commit that referenced this issue Oct 1, 2024
Clarify in the `README.md` file that, although the C backend may be
selected for accelerating the kernel files with Pyccel, this is not
fully working yet. Hence the Fortran backend (which is the default) is
the only one available. A future version of Pyccel will certainly
provide a C backend as capable as the Fortran one. See issue #431.

Co-authored-by: Martin Campos Pinto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working codegen Automatic code generation
Projects
None yet
Development

No branches or pull requests

4 participants