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

IR: Fix get_pragma_params for multiline pragmas #313

Merged
merged 2 commits into from
May 10, 2024

Conversation

mlange05
Copy link
Collaborator

@mlange05 mlange05 commented May 7, 2024

The recent addition of parse_expr for reading pragma parameters does not account for multi-line pragmas with & linebreaks. This PR fixes this and adds a small test for this.

I've also piggy-backed a small fix to loki-transform.py, where the recent introduction of RemoveCodeTransformation was not told to keep DR_HOOK calls in the drivers via the kernel_only option. With these two changes, EC-physics regression should be fine again.

@mlange05 mlange05 requested a review from reuterbal May 7, 2024 15:19
Copy link

github-actions bot commented May 7, 2024

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/313/index.html

@mlange05 mlange05 force-pushed the naml-fix-pragma-params-multiline branch from 9510d06 to 00abd4c Compare May 8, 2024 08:43
@mlange05 mlange05 marked this pull request as draft May 8, 2024 09:31
mlange05 added 2 commits May 10, 2024 14:11
The associated test also checks the associated use of `fgen`, which
triggered this problem in the first place.
@reuterbal reuterbal force-pushed the naml-fix-pragma-params-multiline branch 2 times, most recently from 8486c69 to ddb3e0d Compare May 10, 2024 12:29
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

Many thanks, let's wait for CI tests to confirm but this looks great.

@reuterbal reuterbal marked this pull request as ready for review May 10, 2024 12:45
@reuterbal reuterbal added the ready for merge This PR has been approved and is ready to be merged label May 10, 2024
Copy link

codecov bot commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.11%. Comparing base (5300280) to head (ddb3e0d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #313   +/-   ##
=======================================
  Coverage   95.11%   95.11%           
=======================================
  Files         165      165           
  Lines       35065    35079   +14     
=======================================
+ Hits        33351    33365   +14     
  Misses       1714     1714           
Flag Coverage Δ
lint_rules 96.39% <ø> (ø)
loki 95.09% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@reuterbal reuterbal merged commit 61c93b2 into main May 10, 2024
12 checks passed
@reuterbal reuterbal deleted the naml-fix-pragma-params-multiline branch May 10, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge This PR has been approved and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants