Skip to content

Conversation

@Iximiel
Copy link
Member

@Iximiel Iximiel commented Jul 1, 2025

Description

Hi @gtribello, I am starting also to move some of the functionalities of function to the GPU.

I started with BESSEL but I think there are no test to check if I am doing things correctly, and I am sad since I like that refactor with no need of redeclaring A or B for each instance.

Being more serious: at time of writing this I did the "around trio" : "LESS_THAN BETWEEN MORE_THAN", I think I succeeded in porting them as standalone actions, but I am not sure how it will be possible to tag them with USEGPU in the shortcuts like DISTANCES ATOMS1=1,2 ATOMS2=5,4 BETWEEN1={GAUSSIAN LOWER=0.5 UPPER=1.0 LABEL=b1} BETWEEN2={GAUSSIAN LOWER=1.0 UPPER=2.0 LABEL=b2}. I think that the solution for that will need a separate PR/discussion

Target release

I would like my code to appear in release v2.11

Type of contribution
  • changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
  • changes to a module not authored by you
  • new module contribution or edit of a module authored by you
Copyright
  • I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.
  • the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.
Tests
  • I added a new regtest or modified an existing regtest to validate my changes.
  • I verified that all regtests are passed successfully on GitHub Actions.

@gtribello
Copy link
Member

Hi @Iximiel

I think you have worked this out already, but BESSEL is not something we are likely to need on the GPU. The reason it is implemented is described in the manual:

https://www.plumed.org/doc-master/user-doc/html/BESSEL/

It is tested in the PAMM module.

In terms of getting things like DISTANCES LESS_THAN to work, I wouldn't worry about it. This syntax is deprecated. There are some shortcuts where there might be value in adding the option in symfunc, but we can do them on a case-by-case basis.

@Iximiel
Copy link
Member Author

Iximiel commented Jul 2, 2025

Ok, then I remove the #ifdef openacc things from BESSEL

It is tested in the PAMM module.

If it pass the test there, I would keep it like this, so that it is not initializing/copying extra 55 numbers each time it is called


@carlocamilloni we should try to see if the chbevl function is faster with the constexpr arrays instead of the vectors for A and B in also SAXS, but that is a topic for another PR

@Iximiel Iximiel force-pushed the gpu/functionOfMatVec branch from 0687f06 to 4e60bea Compare July 2, 2025 07:58
@Iximiel Iximiel marked this pull request as ready for review July 2, 2025 07:58
@gtribello
Copy link
Member

HI @Iximiel

To clarify, I think having the OpenACC in Bessel is not going to do any harm. My point was simply there is probably not a lot of people using this action for anything computationally intensive where the GPU is going to make a lot of difference. However, we can change the function and add ACC in there as long as it all just works easily. However, if it is a lot of effort (which I don't think it would be) then it can be abandoned.

@Iximiel
Copy link
Member Author

Iximiel commented Jul 2, 2025

HI @Iximiel

To clarify, I think having the OpenACC in Bessel is not going to do any harm. My point was simply there is probably not a lot of people using this action for anything computationally intensive where the GPU is going to make a lot of difference. However, we can change the function and add ACC in there as long as it all just works easily. However, if it is a lot of effort (which I don't think it would be) then it can be abandoned.

ok, so the code can remain like this, in future you can add the #ifdef __PLUMED_HAS_OPENACC #define __PLUMED_USE_OPENACC 1 #endif //__PLUMED_HAS_OPENACC to reactivate the GPU, as now we are saving some compilation time (nvc is SLOW) and avoiding extra errors.

At this point I think this is ok to be merged, if we merge #1292 before this I can rebase and update gpu.md with the new actions directly here


I found the chain of calls to test BESSEl: To test BESSEL you have to call KERNEL, but to test KERNEL you have to check PAMM. It might be better to have a something more focused on each step

@Iximiel Iximiel force-pushed the gpu/functionOfMatVec branch from 4e60bea to ec6834d Compare July 2, 2025 09:41
@gtribello gtribello merged commit 940f6e8 into plumed:master Jul 7, 2025
21 checks passed
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.

2 participants