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

Complete the Fortran interface and extend the Python interface #256

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

janw20
Copy link
Collaborator

@janw20 janw20 commented Feb 2, 2024

Hi, I was working on using PineAPPL with an existing Fortran code, and during that, I completed the Fortran interface to the PineAPPL C API, wrote a file to test this and added an example how to use the interface with LHAPDF, and also I implemented the convolute_with_two function in the Python interface.

@cschwan
Copy link
Contributor

cschwan commented Feb 7, 2024

Hi @janw20, thank you for your pull request! I had a look at your changes and they look pretty much as they should. I will comment on a few minor details, but after fixing them we can merge the PR quite soon.

examples/fortran/lhapdf_example.f90 Outdated Show resolved Hide resolved
examples/fortran/test.f90 Outdated Show resolved Hide resolved
@cschwan
Copy link
Contributor

cschwan commented Feb 7, 2024

We probably also want to improve the following points (not necessarily in this PR):

@janw20
Copy link
Collaborator Author

janw20 commented Feb 7, 2024

Hi @cschwan, thanks for your comments, I added the terminating newlines. I started learning Rust but I'm not very proficient yet, so I don't know if I can contribute to how the Fortran module would be compiled with cargo-c. But I think it would be nice if it would be installed together with the C API.

@alecandido
Copy link
Member

@cschwan is certainly more expert than myself (he did it) but, ttbomu,the only two things you should be concerned about for C installation are:

[package.metadata.capi.install.include]
asset = [{from = "include/PineAPPL.hpp"}]

and
https://github.com/NNPDF/pineappl/blob/master/pineappl_capi/cbindgen.toml

They are both declarative and pretty much straightforward.

For Fortran I guess there are additional problems, not related to Rust or cargo-c, but to Fortran itself. Most likely, the discussion pointed out by @cschwan is a great starting point (and my guess is that the main issue is the portability of the possible alternatives...)

@cschwan
Copy link
Contributor

cschwan commented Feb 7, 2024

No worries, @janw20, my comment above was more about writing down the ideas so they don't get lost. I created dedicated Issues for them now: #257 and #258.

@cschwan
Copy link
Contributor

cschwan commented Feb 7, 2024

This looks good and I'm merging it now. Thanks again, @janw20!

@cschwan cschwan merged commit 0ca737b into NNPDF:master Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants