PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

(Closes #1664) Add ACCWaitDirective and async keyword on kernels, enter data, parallel

Open arporter opened this issue 1 year ago • 2 comments

This is a replacement/takeover of #2070 as Sebastien has moved to a new job.

arporter avatar Jul 18 '24 15:07 arporter

Now that I look at this again, there don't seem to be any tests of the new directive and keyword for the Fortran psyir backend (it's all gen_code in LFRic).

arporter avatar Jul 18 '24 17:07 arporter

I see from https://github.com/stfc/PSyclone/pull/2070#discussion_r1319986879 that this PR was started before we had ACCClause (#2157). However, we do now have clauses and therefore this code needs updating (to remove the use of Signature) and tests for the Fortran backend are required.

arporter avatar Jul 18 '24 17:07 arporter

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.89%. Comparing base (0a0cfa6) to head (4eecdc2). Report is 32 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #2664    +/-   ##
========================================
  Coverage   99.89%   99.89%            
========================================
  Files         361      362     +1     
  Lines       51368    51571   +203     
========================================
+ Hits        51316    51519   +203     
  Misses         52       52            

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 22 '24 12:11 codecov[bot]

@arporter

  1. I worked on the code coverage. Let's see what the next push brings.
  2. There was something in the code that I didn't understand: async_queue was never to None, but then checked against None. But I found places where async_queue was set to False if the option didn't exist. I changed this throughout the code that async_queue is set per default to None and to an integer otherwise.
  3. Regarding the options dictionary, I set the default argument value to {}. This avoids further checks below ala option is None.

schreiberx avatar Nov 25 '24 22:11 schreiberx

@arporter

1. I worked on the code coverage. Let's see what the next push brings.

2. There was something in the code that I didn't understand:
   `async_queue` was never to `None`, but then checked against `None`.
   But I found places where `async_queue` was set to `False` if the option didn't exist. I changed this throughout the code that `async_queue` is set per default to `None` and to an integer otherwise.

3. Regarding the options dictionary, I set the default argument value to {}. This avoids further checks below ala `option is None`.

I updated the async part in the previous push. async_queue can be finally set to these values:

  • None or False which doesn't make use of it
  • True which uses it without specifying a concrete queue
  • An integer or a Reference as the concrete queue to use.

schreiberx avatar Nov 26 '24 11:11 schreiberx

Ready for another look now @sergisiso.

arporter avatar Mar 11 '25 21:03 arporter