Skip to content

Remove obsolescent and non-standard features in physics modules#1273

Merged
weiwangncar merged 9 commits intowrf-model:release-v4.2.2from
milancurcic:1270-remove-nonstandard-features
Jan 12, 2021
Merged

Remove obsolescent and non-standard features in physics modules#1273
weiwangncar merged 9 commits intowrf-model:release-v4.2.2from
milancurcic:1270-remove-nonstandard-features

Conversation

@milancurcic
Copy link
Contributor

@milancurcic milancurcic commented Aug 17, 2020

TYPE: Bug fix

KEYWORDS: Fix, obsolescent, non-standard, features

SOURCE: Milan Curcic (University of Miami)

DESCRIPTION OF CHANGES:
Problem:
This PR removes or fixes some obsolescent and non-standard features in several physics modules which were
preventing WRF to be built with IBM XL v16.1.1 on Power9, as reported in #1270.

Solution:

  • Replace amax1() occurrences with max()
  • Explicitly cast literal constants that are passed to min() and max() intrinsics so that they are type and kind
    compatible with other arguments
  • Replace calls to the non-standard flush() subroutine with the standard flush() statement.

ISSUE:
Fixes #1270

LIST OF MODIFIED FILES:
M phys/module_cu_mskf.F
M phys/module_cu_scalesas.F
M phys/module_dust_emis.F
M phys/module_mp_fast_sbm.F
M phys/module_mp_milbrandt2mom.F
M phys/module_mp_p3.F
M phys/module_mp_thompson.F
M phys/module_ra_goddard.F
M phys/module_shcu_deng.F

TESTS CONDUCTED:

  1. These fixes were necessary to allow WRF to be built and run on IBM Power9 with the XL v16.1.1 compiler. The
    executables build in both single (./configure) and double (./configure -r8) precision modes.
  2. Jenkins testing is all pass.

RELEASE NOTE: Several physics modules were updated to be more Fortran standard-conforming. This PR removes or fixes some obsolescent and non-standard features in several physics modules which were preventing WRF to be built with IBM XL v16.1.1 on Power9.

@milancurcic milancurcic requested a review from a team as a code owner August 17, 2020 20:32
@dudhia
Copy link
Collaborator

dudhia commented Aug 17, 2020

Dave will need to confirm, but it looks fine to me and probably can be targeted for release 4.2.2.

@milancurcic
Copy link
Contributor Author

Oops, I only now see that I based this off master. We can rebase it to release-v4.2.2 if needed.

@dudhia
Copy link
Collaborator

dudhia commented Aug 17, 2020 via email

@dudhia
Copy link
Collaborator

dudhia commented Aug 17, 2020 via email

@milancurcic milancurcic changed the base branch from master to release-v4.2.2 August 17, 2020 21:32
@milancurcic
Copy link
Contributor Author

@dudhia @weiwangncar @davegill Does this PR need any more care or is it good to go?

@davegill
Copy link
Contributor

@milancurcic

Does this PR need any more care or is it good to go?

Milan,
Do your mods work when building with double precision? Since our compilers do not catch this, we rely on you.

> ./clean -a
> ./configure -r8
> ./compile em_real >& foo

@milancurcic
Copy link
Contributor Author

@davegill Good call. -r8 triggers similar errors in other physics modules. Stand by for more fixes.

@milancurcic milancurcic requested a review from a team as a code owner September 1, 2020 21:42
@milancurcic
Copy link
Contributor Author

Both single and double precision modes now build. I updated the PR description with new files that are changed.

@weiwangncar
Copy link
Collaborator

I'm ok with this PR.

@weiwangncar
Copy link
Collaborator

@davegill @dudhia Any more comments on this PR? Shall I approve and merge it?

@dudhia
Copy link
Collaborator

dudhia commented Dec 29, 2020

I wonder if it should go to develop instead of 4.2.2

@weiwangncar
Copy link
Collaborator

@dudhia @davegill I can agree to that, making this PR to develop branch.

@davegill
Copy link
Contributor

@weiwangncar @dudhia
Folks,
I think that this should be directed at the release-v4.2.2 branch, not develop. This is definitely a bug fix. I am OK with this, but I do not want to approve if you guys are still deciding which branch to use.

I removed the un-associated mod to the .gitignore file. This is now completely a fix for compiling on power9 with IBM's xl compiler.

@weiwangncar
Copy link
Collaborator

@davegill I have approved it. So I'm ok for this to go to 4.2.2.

@dudhia
Copy link
Collaborator

dudhia commented Dec 29, 2020 via email

@davegill
Copy link
Contributor

@dudhia

I just saw a bunch of other developers who might need to be notified for various physics options affected.

Is this a request to not merge the code?

@dudhia
Copy link
Collaborator

dudhia commented Dec 29, 2020 via email

@davegill
Copy link
Contributor

@dudhia @weiwangncar
Jimy,
Understood.

I think that since these are bug fixes, we notify the developers to review this PR. There are some schemes that we can say that "we" own.

@JS-WRF-SBM
Copy link
Contributor

@davegill @milancurcic @dudhia @weiwangncar

Folks, thanks for working on these corrections.
I'm OK with the changes for the phys/module_mp_fast_sbm.F scheme in this PR.

Koby

@gthompsnWRF
Copy link
Contributor

@davegill I am fine with the changes. I see only one change to mp-thompson and 2 changes to module_dust_emis. I would think the latter file could just use all single-prec variables and skip all that casting to double that seems wasteful. Either way, it is no different to me. Thanks.

@llpcarson
Copy link
Contributor

Looks ok to me (module_cu_scalesas.F )

@davegill
Copy link
Contributor

davegill commented Jan 7, 2021

@dudhia @weiwangncar
Folks,
We have heard back from almost all impacted. In addition to those above, Hugh, Janson, and Kiran sent "OK" emails. I say we go for it with these mods.

@weiwangncar
Copy link
Collaborator

I'm ok with this, and have already approved this PR.

@davegill
Copy link
Contributor

davegill commented Jan 7, 2021

@dudhia @weiwangncar
Jimy,
Wei has already approved this. Most of the impacted physics developers have given a thumbs up. This is OK to merge

@dudhia
Copy link
Collaborator

dudhia commented Jan 7, 2021 via email

@weiwangncar weiwangncar merged commit 062b451 into wrf-model:release-v4.2.2 Jan 12, 2021
vlakshmanan-scala pushed a commit to scala-computing/WRF that referenced this pull request Apr 4, 2024
…model#1273)

TYPE: Bug fix

KEYWORDS: Fix, obsolescent, non-standard, features

SOURCE: Milan Curcic (University of Miami)

DESCRIPTION OF CHANGES:
Problem:
This PR removes or fixes some obsolescent and non-standard features in several physics modules which were
preventing WRF to be built with IBM XL v16.1.1 on Power9, as reported in wrf-model#1270.

Solution:
1. Replace amax1() occurrences with max()
2. Explicitly cast literal constants that are passed to min() and max() intrinsics so that they are type and kind
compatible with other arguments
3. Replace calls to the non-standard flush() subroutine with the standard flush() statement.

ISSUE:
Fixes wrf-model#1270

LIST OF MODIFIED FILES:
M phys/module_cu_mskf.F
M phys/module_cu_scalesas.F
M phys/module_dust_emis.F
M phys/module_mp_fast_sbm.F
M phys/module_mp_milbrandt2mom.F
M phys/module_mp_p3.F
M phys/module_mp_thompson.F
M phys/module_ra_goddard.F
M phys/module_shcu_deng.F

TESTS CONDUCTED:

These fixes were necessary to allow WRF to be built and run on IBM Power9 with the XL v16.1.1 compiler. The
executables build in both single (./configure) and double (./configure -r8) precision modes.
Jenkins testing is all pass.

RELEASE NOTE: Several physics modules were updated to be more Fortran standard-conforming. This PR removes or fixes some obsolescent and non-standard features in these physics modules which were preventing WRF to be built with IBM XL v16.1.1 on Power9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Obsolescent and non-standard features in several physics modules

7 participants