GEOS-ESM / MAPL

MAPL is a foundation layer of the GEOS architecture, whose original purpose is to supplement the Earth System Modeling Framework (ESMF)
https://geos-esm.github.io/MAPL/
Apache License 2.0
26 stars 17 forks source link

Need Update of ACG for Proper Handling of Aerosol Longnames #2847

Closed acollow closed 3 weeks ago

acollow commented 4 months ago

the issue

For carbonaceous aerosols the State Spec reads MASS | kg kg-1 | xyz | C | | Aerosol Mass Mixing Ratio

which creates the short name BCMASS and long name CA.bc Aerosol Mass Mixing Ratio.

the fix

Please modify the automatic code generator so that the long name reads as "BC Aerosol Mass Mixing Ratio". Please consult with @amdasilva if any questions.

This is needed for M21C and GEOS FP.

mathomp4 commented 4 months ago

First, I've added @darianboggs as he is our ACG expert.

But, I think this is a deliberate choice by GOCART2G (and I hope @amdasilva can weigh in).

This line (say) in CA2G_StateSpecs.rc:

 *EXTCOEF     | m-1          | xyz   | C     | size(self%wavelengths_profile) | * Aerosol Extinction Coefficient

when processed by the ACG substitutes the component name for the *. And per the instance definitions in GOCART2G_GridComp.rc, the component names are CA.bc, CA.oc, and CA.br.[^1]

The only reason, you get BCEXTCOEF in a history output is because it is renamed in HISTORY.rc from CA.bcEXTCOEF to BCEXTCOEF:

         'CA.bcEXTCOEF'     , 'CA.bc'    , 'BCEXTCOEF'  ,

But, as the ACG currently works, the long name is based on the component name so you get:

                BCEXTCOEF:long_name = "CA.bc Aerosol Extinction Coefficient" ;

I suppose we could put in a translation table into the ACG so that it knows CA.bc is BC. But to me that seems to be outside the purview of MAPL as we'd be encoding GOCART-specific information into a generic MAPL script. GOCART itself decided that the name of the black carbon instance of the Carbonaceous aerosol gridded component is CA.bc and MAPL is honoring that.

One possible path[^2] I can see that keeps the separation of concerns to update the long name is perhaps putting the override information inside of the CA GridComp itself and then having the ACG ingest that. This would require:

  1. Creating and adding the info in CA (either as RC or YAML) with the correct override (aka CA.bc long name = 'Black Carbon')
  2. Updating the ACG with a command line argument to use this translation table and "override" the comp_name used in mangling the long name info
  3. Updating the CMake code that runs the ACG with an option that adds the command line option.

But, I guess we'd need affirmation from @amdasilva (for GOCART) and @tclune (for MAPL) that this is the right way forward to do this.

[^1]: Internal to the CA Grid Comp there is a sort of translation of the component names, but as far as MAPL and GOCART are concerned, the Gridded Component running is called, e.g., CA.bc [^2]: The other path is having GEOS-FP and M21C run ncatted a posteriori on the collections. Not hard, but then it's another postprocessing script.

tclune commented 4 months ago

I concur with Matt's analysis. To really do this "right" would require introducing a bit more grammar into the ACG. Probably would not need to be "GOCART specific", but we'd need something a bit more complex than expanding "*" in the current manner.

I'll note that in History3g, we probably already have an acceptable solution (but incomplete implementation) in that the history collection can override the long name and not just given an alias as with current History. And putting that back capability into the current history is problematic only because the list

         'CA.bcEXTCOEF'     , 'CA.bc'    , 'BCEXTCOEF'  ,

is already highly positional. (3g uses yaml which gives us clearer mechanisms for such customizations). But we could in principle but an alternate long name in the 5th column in the collection. Including @bena-nasa so that he could comment on how useful/difficult this extension to current history is.

mathomp4 commented 4 months ago

Okay. I think we have a possible solution. I've discussed with @darianboggs and we might be "in luck".

First, my thought above cannot work. Why? well as @darianboggs reminded me, all the ACG does is make Fortran code. So, it makes, for example:

   call MAPL_AddExportSpec(gc,&
        & short_name=''//trim(comp_name)//'MASS', &
        & units='kg kg-1', &
        & dims=MAPL_DimsHorzVert, &
        & vlocation=MAPL_VlocationCenter, &
        & long_name=''//trim(comp_name)//' Aerosol Mass Mixing Ratio', &
        & _RC)

So, to have the long_name be something different, the Fortran code must know what that something different is. But, we are in luck. In CA2G we have this chunk of code:

    if (comp_name(1:5) == 'CA.oc') then
       GCsuffix = 'OC'
    else if (comp_name(1:5) == 'CA.bc') then
       GCsuffix = 'BC'
    else if (comp_name(1:5) == 'CA.br') then
       GCsuffix = 'BR'
    end if

So, what I'm proposing is we add a new option to the ACG, something like:

... --long_name_prefix "GCsuffix" ...

and then the ACG says, oh, we have that option set so instead of mangling the long_name with trim(comp_name) we mangle it with that variable: trim(GCsuffix):

   call MAPL_AddExportSpec(gc,&
        & short_name=''//trim(comp_name)//'MASS', &
        & units='kg kg-1', &
        & dims=MAPL_DimsHorzVert, &
        & vlocation=MAPL_VlocationCenter, &
        & long_name=''//trim(GCsuffix)//' Aerosol Mass Mixing Ratio', &
        & _RC)

@darianboggs can add the Python option and then I can fix up the CMake.

The "nice" thing about this is that the only GOCART code that need change is the CMakeLists.txt where a new option needs to be passed to mapl_acg.

Is this portable? Sort of? At least in the sense that someone has to add code to the GridComp which has a translation, but that seems to be the only way.

acollow commented 4 months ago

@mathomp4 , that block of code in CA2g_Gridcomp was exactly what I was thinking of when I was talking with Ben about trimming the component!

mathomp4 commented 4 months ago

@acollow Thanks to @darianboggs we have a PR (#2858) that works for me. I get out, say:

        float BCCMASS(time, lat, lon) ;
                BCCMASS:_FillValue = 1.e+15f ;
                BCCMASS:add_offset = 0.f ;
                BCCMASS:fmissing_value = 1.e+15f ;
                BCCMASS:long_name = "BC Aerosol Column Mass Density" ;
                BCCMASS:missing_value = 1.e+15f ;
                BCCMASS:regrid_method = "conserve" ;
                BCCMASS:scale_factor = 1.f ;
                BCCMASS:standard_name = "BC Aerosol Column Mass Density" ;
...

where BCCMASS is due to a history rename and the long_name is changed via a CMake change in CA.

Once @tclune can take a look at it, we can get this into develop with the PR, but what we need to know from you is what other branches/versions of MAPL need it.

I suppose that would be R21C for M21C but I'm not sure which FP would want this? Upcoming FP from @rtodling? Or a current ops tag?

acollow commented 4 months ago

Awesome! Thank you to the SI Team for working on this! It will need to go into the R21C branch (and ultimately the next M21C tag) and a future FP. It would be great to have @rtodling include it in an x run so I can verify it there as well. It is NOT needed for GEOS-IT or the currently operational FP as those systems do not include GOCART-2G.

@mathomp4 , Note that M21C will also need the MAPL patch that corrects the "001" bug.

mathomp4 commented 4 months ago

Awesome! Thank you to the SI Team for working on this! It will need to go into the R21C branch (and ultimately the next M21C tag) and a future FP. It would be great to have @rtodling include it in an x run so I can verify it there as well. It is NOT needed for GEOS-IT or the currently operational FP as those systems do not include GOCART-2G.

Okay. I'll work on a PR to get it into R21C once it's settled. And @rtodling do you know what version of MAPL I should look at for the FP ?

@mathomp4 , Note that M21C will also need the MAPL patch that corrects the "001" bug.

I vaguely remember that. I'll talk with @atrayano because I think he fixed that

github-actions[bot] commented 4 weeks ago

This issue has been automatically marked as stale because it has not had activity in the last 60 days. If there are no updates within 7 days, it will be closed. You can add the ":hourglass: Long Term" label to prevent the stale action from closing this issue.