Noah-MP Dynamic Irrigation Packaged 2d fields by prasanthvkrishna · Pull Request #1456 · wrf-model/WRF

prasanthvkrishna · 2021-04-04T20:54:38Z

TYPE: bug fix

KEYWORDS: Noah MP dynamic irrigation packaging 2d variables.

SOURCE: Prasanth Valayamkunnath (NCAR)

DESCRIPTION OF CHANGES:
Purpose: To package Noah MP dynamic irrigation variables.

LIST OF MODIFIED FILES:
Registry/Registry.EM_COMMON
Registry/Registry.NMM
phys/module_sf_noahmplsm.F (resolved issue with ISNAN in PGI)

TESTS CONDUCTED:

Compiled the model using intel/19.0.5. No further testing is conducted as these are minor changes in the code.
Jenkins testing OK

dudhia · 2021-04-05T15:14:33Z

Typically packaging needs a test to make sure not using this option is unaffected. If the arrays are still accessed in any way when the option is off, it could cause problems with unallocated space. Make sure these arrays are not in any executed code lines when the option is off.

phys/module_sf_noahmplsm.F

prasanthvkrishna · 2021-04-05T17:57:55Z

Typically packaging needs a test to make sure not using this option is unaffected. If the arrays are still accessed in any way when the option is off, it could cause problems with unallocated space. Make sure these arrays are not in any executed code lines when the option is off.

Sure, I will do a test. Thanks, Jimy

davegill · 2021-04-05T18:45:44Z

@prasanthvkrishna @cenlinhe

ISNAN is not supported by PGI compiler

This intrinsic became part of the standard with Fortran2003. We should be able to rely on isnan being present (not a slam on you, but on the PGI compiler that you are using).

I will defer to your decision.

prasanthvkrishna · 2021-04-05T18:53:22Z

@prasanthvkrishna @cenlinhe

ISNAN is not supported by PGI compiler

This intrinsic became part of the standard with Fortran2003. We should be able to rely on isnan being present (not a slam on you, but on the PGI compiler that you are using).

I will defer to your decision.
@davegill
So you suggest using ISNAN? Should I make it active then?

jamiebresch · 2021-04-05T18:53:27Z

@prasanthvkrishna @cenlinhe

ISNAN is not supported by PGI compiler

This intrinsic became part of the standard with Fortran2003. We should be able to rely on isnan being present (not a slam on you, but on the PGI compiler that you are using).

I will defer to your decision.

isnan can be implemented like

WRF/phys/module_mp_p3.F

Lines 5311 to 5315 in fb60d61

    
           logical function isnan(arg1) 
        
                 real,intent(in) :: arg1 
        
                 isnan=( arg1  .ne. arg1 ) 
        
                 return 
        
           end function isnan

That function appears not actually used by P3, though.

cenlinhe · 2021-04-05T18:54:59Z

@davegill @prasanthvkrishna Thanks. Not sure why the PGI compiler in Cheyenne does not include this intrinsic (I tested it using the latest PGI compiler pgi/20.4). How about this: let's remove this ISNAN in noahmplsm here for this WRF release, if we find any potential NaN issue that might occur to this variable (less likely) in the future applications, we will revisit it through bug fix.

prasanthvkrishna · 2021-04-05T18:55:43Z

@prasanthvkrishna @cenlinhe

ISNAN is not supported by PGI compiler

This intrinsic became part of the standard with Fortran2003. We should be able to rely on isnan being present (not a slam on you, but on the PGI compiler that you are using).
I will defer to your decision.

isnan can be implemented like

WRF/phys/module_mp_p3.F

Lines 5311 to 5315 in fb60d61

logical function isnan(arg1)

real,intent(in) :: arg1

isnan=( arg1 .ne. arg1 )

return

end function isnan

That function appears not actually used by P3, though.

@jamiebresch Thanks :)

cenlinhe · 2021-04-05T18:56:19Z

@prasanthvkrishna @cenlinhe

ISNAN is not supported by PGI compiler

This intrinsic became part of the standard with Fortran2003. We should be able to rely on isnan being present (not a slam on you, but on the PGI compiler that you are using).
I will defer to your decision.

isnan can be implemented like

WRF/phys/module_mp_p3.F

Lines 5311 to 5315 in fb60d61

logical function isnan(arg1)

real,intent(in) :: arg1

isnan=( arg1 .ne. arg1 )

return

end function isnan

That function appears not actually used by P3, though.

@jamiebresch Thanks :)

This would also be an option.

cenlinhe · 2021-04-05T18:58:49Z

@prasanthvkrishna @davegill OK, it seems that we could implement the isnan function to resolve this issue. Prasanth, could you please implement this function into module_sf_noahmplsm.F and do a quick test if it works?

prasanthvkrishna · 2021-04-05T19:00:38Z

Sure, I will do that.

…

On Mon, Apr 5, 2021 at 6:59 PM Cenlin_He ***@***.***> wrote: @prasanthvkrishna <https://github.com/prasanthvkrishna> @davegill <https://github.com/davegill> OK, it seems that we could implement the isnan function to resolve this issue. Prasanth, could you please implement this function into module_sf_noahmplsm.F and do a quick test if it works? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1456 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHR7VP35MFXM3C7XLNN6PSTTHIB7PANCNFSM42LY3H5Q> .

cenlinhe · 2021-04-05T19:13:55Z

@davegill So if we implement the logical function as suggested above following module_mp_p3.F, we will use another function name "isnan_lsm" to avoid conflicts with "isnan" that is already an intrinsic function name for INTEL currently. Do you think this works for you?

davegill · 2021-04-07T17:04:14Z

@prasanthvkrishna @dudhia @weiwangncar
Prasanth,
Take a look at #1460
What if we removed the ISNAN part of this PR, and put the ISNAN change into my PR?

davegill · 2021-04-07T17:38:52Z

phys/module_sf_noahmplsm.F

+
+    logical function isnan_lsm(arg1)
+        real,intent(in) :: arg1
+       isnan_lsm = (arg1 .ne. arg1)
+       return
+    end function isnan_lsm
+


This "roll your own" ISNAN() function is the technique used in other physics packages (P3 MP, TEMF BL). I am entirely OK with this solution.

Hi @davegill, OK thanks. We will go with our solution here by defining the isnan_lsm function as done in other physics packages.

davegill · 2021-04-07T17:40:54Z

@davegill So if we implement the logical function as suggested above following module_mp_p3.F, we will use another function name "isnan_lsm" to avoid conflicts with "isnan" that is already an intrinsic function name for INTEL currently. Do you think this works for you?

Yes, this is just fine. It is done elsewhere in the model.

davegill

Packaging is a good idea
ISNAN() functionality is implemented as is already done in P3 and TEMF

cenlinhe · 2021-04-07T17:47:46Z

Thanks, @davegill !

prasanthvkrishna · 2021-04-07T17:48:32Z

Packaging is a good idea

ISNAN() functionality is implemented as is already done in P3 and TEMF

@davegill Thanks, Dave.

davegill · 2021-04-08T17:47:42Z

@prasanthvkrishna @cenlinhe @dudhia

Make sure these arrays are not in any executed code lines when the option is off.

Folks,
Jimy made the request above. Has this been verified?

prasanthvkrishna · 2021-04-08T17:58:35Z

@prasanthvkrishna @cenlinhe @dudhia

Make sure these arrays are not in any executed code lines when the option is off.

Folks,
Jimy made the request above. Has this been verified?

@dudhia @davegill Yes Dave. I tested it by opting the unified Noah instead of Noah-MP. None of the dynamic irrigation 2d fields are in the WRF output. So, the packaging worked.

dudhia · 2021-04-08T18:01:29Z

Another test should make sure NoahMP results are not changed when the irrigation scheme is off before and after packaging.

…

On Thu, Apr 8, 2021 at 11:58 AM Prasanth Valayamkunnath < ***@***.***> wrote: @prasanthvkrishna <https://github.com/prasanthvkrishna> @cenlinhe <https://github.com/cenlinhe> @dudhia <https://github.com/dudhia> Make sure these arrays are not in any executed code lines when the option is off. Folks, Jimy made the request above. Has this been verified? @dudhia <https://github.com/dudhia> @davegill <https://github.com/davegill> Yes Dave. I tested it by opting the unified Noah instead of Noah-MP. None of the dynamic irrigation 2d fields are in the WRF output. So, the packaging worked. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1456 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77E7JJSGSIERHYE5DRDTHXVF3ANCNFSM42LY3H5Q> .

davegill · 2021-04-13T21:56:06Z

@prasanthvkrishna

Another test should make sure NoahMP results are not changed when the
irrigation scheme is off before and after packaging.

What is the status of Jimy's requested test?

prasanthvkrishna · 2021-04-14T01:39:46Z

@prasanthvkrishna

Another test should make sure NoahMP results are not changed when the
irrigation scheme is off before and after packaging.

What is the status of Jimy's requested test?

@dudhia @davegill
I checked the results. No difference between versions. Thanks.

TYPE: bug fix KEYWORDS: Noah MP, dynamic irrigation, packaging SOURCE: Prasanth Valayamkunnath (NCAR) DESCRIPTION OF CHANGES: 1. To package Noah MP dynamic irrigation variables (small clean-up before release). 2. Generate an internal ISNAN() function. This is already done with TEMF and P3 schemes. LIST OF MODIFIED FILES: Registry/Registry.EM_COMMON Registry/Registry.NMM phys/module_sf_noahmplsm.F (resolve issue with ISNAN with PGI compiler) TESTS CONDUCTED: 1. Compiled the model using intel/19.0.5. 2. Code builds with PGI compiler. 3. Results without NoahMP are identical before vs after. 4. Results with NoahMP are identical before vs after. 5. Jenkins testing OK

prasanthvkrishna added 2 commits April 4, 2021 12:35

packaged dyn irri noahmp

8fe7d99

packaged dyn irri noahmp for nmm

d401518

prasanthvkrishna requested review from a team as code owners April 4, 2021 20:54

prasanthvkrishna mentioned this pull request Apr 4, 2021

Add dynamic irrigation management practices to Noah-MP #1372

Merged

davegill reviewed Apr 5, 2021

View reviewed changes

phys/module_sf_noahmplsm.F Outdated Show resolved Hide resolved

davegill added bug PHYSICS release-v4.3 labels Apr 5, 2021

prasanthvkrishna added 2 commits April 6, 2021 11:27

corrected isnan

604f7f1

ireloss to 100 fromm 1000

de90022

davegill reviewed Apr 7, 2021

View reviewed changes

davegill self-requested a review April 7, 2021 17:41

davegill approved these changes Apr 7, 2021

View reviewed changes

davegill mentioned this pull request Apr 7, 2021

CPP directive around use of ISNAN() intrinsic function #1460

Closed

davegill merged commit 6e29bca into wrf-model:release-v4.3 Apr 14, 2021

Conversation

prasanthvkrishna commented Apr 4, 2021 • edited by davegill Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dudhia commented Apr 5, 2021

Uh oh!

Uh oh!

prasanthvkrishna commented Apr 5, 2021

Uh oh!

davegill commented Apr 5, 2021

Uh oh!

prasanthvkrishna commented Apr 5, 2021

Uh oh!

jamiebresch commented Apr 5, 2021

Uh oh!

cenlinhe commented Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prasanthvkrishna commented Apr 5, 2021

Uh oh!

cenlinhe commented Apr 5, 2021

Uh oh!

cenlinhe commented Apr 5, 2021

Uh oh!

prasanthvkrishna commented Apr 5, 2021 via email

Uh oh!

cenlinhe commented Apr 5, 2021

Uh oh!

davegill commented Apr 7, 2021

Uh oh!

davegill Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

cenlinhe Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

davegill commented Apr 7, 2021

Uh oh!

davegill left a comment

Choose a reason for hiding this comment

Uh oh!

cenlinhe commented Apr 7, 2021

Uh oh!

prasanthvkrishna commented Apr 7, 2021

Uh oh!

davegill commented Apr 8, 2021

Uh oh!

prasanthvkrishna commented Apr 8, 2021

Uh oh!

dudhia commented Apr 8, 2021 via email

Uh oh!

davegill commented Apr 13, 2021

Uh oh!

prasanthvkrishna commented Apr 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

prasanthvkrishna commented Apr 4, 2021 •

edited by davegill

Loading

cenlinhe commented Apr 5, 2021 •

edited

Loading