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

Add SetVm in ESMF/UFS/Aerosol_Cap.F90 for ESMF managed threading capability. #165

Closed
junwang-noaa opened this issue Jul 1, 2022 · 8 comments · Fixed by #166
Closed

Add SetVm in ESMF/UFS/Aerosol_Cap.F90 for ESMF managed threading capability. #165

junwang-noaa opened this issue Jul 1, 2022 · 8 comments · Fixed by #166
Assignees

Comments

@junwang-noaa
Copy link

We are adding the ESMF managed threading in UFS weather model. Two lines of code need to be added to ESMF/UFS/Aerosol_Cap.F90.

diff --git a/ESMF/UFS/Aerosol_Cap.F90 b/ESMF/UFS/Aerosol_Cap.F90
index ad6dc04..d44ab1f 100644
--- a/ESMF/UFS/Aerosol_Cap.F90
+++ b/ESMF/UFS/Aerosol_Cap.F90
@@ -6,6 +6,7 @@ module Aerosol_Cap
   use NUOPC
   use NUOPC_Model, only : &
     NUOPC_ModelGet, &
+    SetVM,  &
     model_routine_SS            => SetServices,          &
     model_routine_Run           => routine_Run,          &
     model_label_Advance         => label_Advance,        &
@@ -75,6 +76,7 @@ module Aerosol_Cap
   private

   public :: SetServices
+  public :: SetVM

 contains

Could these two lines be added in the GOCART code? We expect no impact on the GOCART that does not use the ESMF managed threading. Thanks.

@junwang-noaa
Copy link
Author

@tclune @weiyuan-jiang @theurich FYI.

@tclune
Copy link
Contributor

tclune commented Jul 1, 2022

@junwang-noaa I'm sure we can add those lines - but it may take some time before it appears in a release.

At first glance, it would appear that SetVM is not used, so I don't really see why these lines would matter. Is it anticipation of further changes where SetVM is used? In which case, maybe it is better to wait for those?

@junwang-noaa
Copy link
Author

junwang-noaa commented Jul 1, 2022

@tclune This is the exact question I had. Here is the answer from Gerhard: "we do still need the public SetVM for GOCART. The reason is that it is through this mechanism the GOCART code runs on exactly the same PETs as the threaded FV3 FCST component. This is important for the reference sharing between FCST and GOCART. It would not otherwise work under the ESMF-managed setup, where each component decides independently of how to use the PEs provided by the parent component.
However, the GOCART change is so minor, it should not affect what NASA is doing. "

@theurich
Copy link

theurich commented Jul 1, 2022

@tclune The SetVM is unsed under UFS. Notice that the GOCART cap change use associates SetVM with the generic version from NUOPC_Model, and makes it public. This is how the UFS driver can access it. This generic SetVM looks at the defined NUOPC attributes, to handle the resource management for the component.

@tclune
Copy link
Contributor

tclune commented Jul 1, 2022

OK - so you are saying that the Aerosol_Cap is simply re-exporting a NUOPC entity so that some other layer (GOCART CAP) has access. Wouldn't it make more sense to put the USE statement in GOCART CAP instead? Seems odd to put it in a module that does not do anything with it.

@theurich
Copy link

theurich commented Jul 1, 2022

It's the UFS driver that does something with it. I don't expect the GOCART CAP wanting to access it, at least not for now. It's along the path of how GOCART is accessed as a NUOPC component under UFS, so it sits in GOCART's NUOPC cap.

@mathomp4
Copy link
Member

mathomp4 commented Jul 1, 2022

@junwang-noaa Please see #166

@junwang-noaa
Copy link
Author

@mathomp4 Great, thank you!

@mathomp4 mathomp4 linked a pull request Jul 1, 2022 that will close this issue
vbuchard added a commit that referenced this issue Jul 6, 2022
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 a pull request may close this issue.

5 participants