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

#1 Add OpenACC compile option to autotools #282

Merged
merged 12 commits into from
May 9, 2024

Conversation

mlee03
Copy link

@mlee03 mlee03 commented Mar 29, 2024

In this PR,

  1. fregrid_acc and libfrencutils_acc directories have been created. The names of the files and the functions within have been changed with the suffix _acc. All OpenACC development for OpenACC enabled fregrid will occur in these two directories.
  2. configure.ac and Makefile.am have been modified to append -acc compiler flag to FCFLAGS and to compile fregrid_acc

Copy link
Contributor

@rem1776 rem1776 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by the copied over libfrencutils stuff. Is libfrencutils_acc a full copy of all the routines in the original libfrencutils with any gpu modifications needed? or is it supposed to only contain routines that were modified, with fregrid_acc using original versions when applicable?

I don't see fregrid_acc calling anything with the _acc added, so is it just using the orignal libfrencutils?

tools/fregrid_acc/CHANGELOG Outdated Show resolved Hide resolved
@mlee03
Copy link
Author

mlee03 commented Apr 8, 2024

@rem1776 currently in this PR, all files in the directories fregrid_acc and libfrencutils_acc are copies of those in fregrid and libfrencutils directories. The only difference is that the functions in the *_acc files have the suffix "_acc".
The compiled executablesfregrid and fregrid_acc at this point should be identical. The OpenACC changes will be merged in in a stepwise fashion to make the reviewing process and backtracing easier.

@rem1776
Copy link
Contributor

rem1776 commented Apr 9, 2024

@rem1776 currently in this PR, all files in the directories fregrid_acc and libfrencutils_acc are copies of those in fregrid and libfrencutils directories. The only difference is that the functions in the *_acc files have the suffix "_acc". The compiled executablesfregrid and fregrid_acc at this point should be identical. The OpenACC changes will be merged in in a stepwise fashion to make the reviewing process and backtracing easier.

So are you going to be removing the unused/unmodified routines in libfrencutils_acc as part of those updates?

If thats the case then i think we should probably only add those routines to begin with, maybe we can just add the acc versions to the same libfrencutils files instead of creating a copy? Saves us the confusion of having copies of things, and having to add a lot of code just to delete it since it'll all still be in the git history.

@mlee03
Copy link
Author

mlee03 commented Apr 9, 2024

Got it got it, yup I was going to remove functions as we go along (also thought it would be easier for debugging), but it is needless code that has to be carried around and removed functions that are needed in the future can be added back in. Okay, will remove all repetitive functions in fregrid_acc and libfrencutils_acc

@mlee03
Copy link
Author

mlee03 commented Apr 11, 2024

@rem1776 Perhaps it's ready. Decided to start almost from scratch.

rem1776
rem1776 previously approved these changes Apr 16, 2024
Copy link
Contributor

@rem1776 rem1776 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! thanks for making those changes i hope they weren't too annoying.

After this goes in I have an m4 file in the gpu_dev branch for the configure checks with the openacc flag, I'll make a PR for it. It'll just adjust the flag for different compilers if needed and make sure it works.

@mlee03
Copy link
Author

mlee03 commented Apr 17, 2024

No problem at all, I think this might be a better start for fregrid_acc. @bensonr @nikizadehgfdl @thomas-robinson @ceblanton, I think this PR is ready for your approvals 😇

@mlee03 mlee03 changed the title Add OpenACC compile option to autotools #1 Add OpenACC compile option to autotools Apr 18, 2024
configure.ac Show resolved Hide resolved
configure.ac Show resolved Hide resolved
README.md Show resolved Hide resolved
@rem1776 rem1776 merged commit f886213 into NOAA-GFDL:master May 9, 2024
4 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.

3 participants