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

297 omp sentinels #326

Merged
merged 5 commits into from
Jul 18, 2024
Merged

297 omp sentinels #326

merged 5 commits into from
Jul 18, 2024

Conversation

hiker
Copy link
Collaborator

@hiker hiker commented Jul 17, 2024

Adds some support for OMP sentinels (#297), which are required for compiling lfric_atm.

The next release of fparser adds support for sentinels to fparser, so this change can then be removed again (#todo in place to remind us, so I would also leave #297 open), we only need to trigger this behaviour in fparser (which means it will not only work for declaration statements). We need this patch for now to continue LFRic compilation (but the source code in question is UM, so I added label UM, and not LFRic).

@hiker hiker added enhancement New feature or request um labels Jul 17, 2024
Copy link
Collaborator

@MatthewHambley MatthewHambley left a comment

Choose a reason for hiding this comment

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

What's the timescale for the fParser fix and the need for directive support in Fab?

source/fab/parse/fortran.py Show resolved Hide resolved
@hiker
Copy link
Collaborator Author

hiker commented Jul 17, 2024

Ready for next review.

@hiker hiker requested a review from MatthewHambley July 17, 2024 23:59
@hiker
Copy link
Collaborator Author

hiker commented Jul 18, 2024

What's the timescale for the fParser fix and the need for directive support in Fab?

I've just submitted my last additional PR that I wanted for the next release, and there seems to be only one issue that might need to be handled before the next release. An fparser release is usually done very quickly, so I would hope (depending on priorities) to get this in maybe a month?

I originally intended not to push this PR to the official repo at all, but we had to integrate all these changes into our PRs (it became very messy to keep track and merging things), so it simplifies things a bit for us. E.g. #301 has this patch and it's not easy to remove it (conflicts).

Copy link
Collaborator

@MatthewHambley MatthewHambley left a comment

Choose a reason for hiding this comment

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

I've raised #327 to remove these changes when they are no longer needed.

@MatthewHambley MatthewHambley merged commit 77cb175 into MetOffice:master Jul 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request um
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants