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

Prepare OPM for DUNE 2.10 #5585

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

blattms
Copy link
Member

@blattms blattms commented Sep 7, 2024

DUNE_VERSION_NEWER has been removed. Falling back to DUNE_VERSION_GTE which exists since at least DUNE 2.7.

Removed switches for older version than 2.7.

@akva2
Copy link
Member

akva2 commented Sep 9, 2024

So this is basically the same thing i do in the bump to 2.9 required, except the other way around, ie, it keeps support for older dune.

@blattms
Copy link
Member Author

blattms commented Sep 9, 2024

So this is basically the same thing i do in the bump to 2.9 required, except the other way around, ie, it keeps support for older dune.

I am not sure I understand this comment. The macro DUNE_VERSION_GTE is there from at least 2.7. Hence there is no bumping here. We just make sure that it also compiles with 2,10 which dropped the DUNE_VERSION_NEWER macro.

Note that DUNE_VERSION_NEWER means the at least the specified version is required. SO NEWER is a bit misleading. If that is 2.7 then the version needs to be >=2.7

@akva2
Copy link
Member

akva2 commented Sep 9, 2024

The conditions you introduce is the same starting in dune 2.8, so you basically add stuff to handle 2.7. i just removed the 2.7 support thus we need none of the DUNE_VERSION_NEWER / DUNE_VERSION_GTE stuff. Strictly speaking my PR bumps to 2.8 required, but I thought we'd go for 2.9 since that is the version we use on every platform (ignoring the temporary rh7 2.8 for equinor).

@blattms blattms marked this pull request as draft September 9, 2024 12:21
@blattms
Copy link
Member Author

blattms commented Sep 9, 2024

Making this draft until i have a compiling version

@blattms
Copy link
Member Author

blattms commented Sep 10, 2024

I still don't get it. I test >=2.7 (GTE)and not >2.7 (GT).

Anyway, as it turns out we do not compile with 2.7 anyway because of #5595

@akva2
Copy link
Member

akva2 commented Sep 10, 2024

I know that. As I said, it's the opposite approach of mine.
I removed existing checks because I dropped 2.7 support. You add additional checks to make 2.7 still work (if we ignore the other issue).

DUNE_VERSION_NEWER has been removed. Falling back to DUNE_VERSION_GTE
which exists since at least DUNE 2.7.

Removed switches for older version than 2.7.
Grid::CollectiveCommunication is gone in 2.10
This works for DUNE 2.7-2.10, but using HAVE_UMFPACK won't work for
older version due to changes needed for 2.10.
Those versions lack Grid::Communication.
@blattms blattms marked this pull request as ready for review October 3, 2024 10:05
@blattms
Copy link
Member Author

blattms commented Oct 3, 2024

I see. Well, it now works with 2,7 -2.10 to make @totto82 happy.

@blattms blattms added this to the Release 2024.10 milestone Oct 3, 2024
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