search

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
25 stars 18 forks source link

Create a list of GCs and introduce new timers for each legacy timer. Where possible, consult GC owner to refine choices. #1214

Closed tclune closed 2 years ago

tclune commented 2 years ago

Technically not a MAPL issue - but this is a project in MAPL.

bena-nasa commented 2 years ago

Here are the gridComp directories, note that some of these contain multiple gridcomps in a MAPL sense.

tclune commented 2 years ago

You got rid of the formatting that lets us treat that as a check list!

tclune commented 2 years ago

oh - or I never saved mine ...

mathomp4 commented 2 years ago

@tclune Query since I probably wasn't in on a lot of the conversations about this: Is there a reason we don't just change, say, MAPL_TimerOn→MAPL_GenericStateOn calls here in MAPL to use the new calls? Maybe protected by a CAP.rc setting or FLAP switch where if it's ON we use profiler%start and if it's off we use the old code?

It looks like there is something there that was tried (though not protected by a switch):

https://github.com/GEOS-ESM/MAPL/blob/ed3b61450cf18600186cbec6260d68edb14c3ace/generic/MAPL_Generic.F90#L5492-L5496

I'm only concerned that it might take many months to go through all of these components and get people to sign off on adding a bunch of calls (and then maybe a while to get them to sign off on removing the old calls).

Is this due to NOAA/UFS or an external partner?

bena-nasa commented 2 years ago

It does seems like changing MAPL_TimerAdd/On/Off to choose which profiler to use seems simple and safe as far as external users are concerned if it defaults to the old timers, internally in the GMAO we could use the new. The new timers can provide all the same information and more. And this will be exponentially easier.

Internally in the GMAO, my intuition (of course my intuition could be wrong) says other than Bill and the SI team, no one looks hard and frequently at these (I'm not say no one else looks at these, just not with the same frequency of scrutiny). So as a first step, as long as we make sure we provide at minimum the same information from the old timers by defaults I don't see a downside other than some scripts may have to change. At the very least this would be a simple experiment.

tclune commented 2 years ago

The only obstacle here (AFAIK) is that the old timers have "manual punctuation" in their names to manage nesting and indentation in the reporting. E.g., " -- foo". The new profiler handles this aspect automatically. If we leave the names "as is" then we end up with ugly formatting. Not the end of the world, but seems not quite the thing we want to do. Certainly not opposed to this as an intermediate step.

OK - grep'ing for actual usage suggests that the call to the new timer can just strip off leading hyphens. There are no spaces. So this might be very easy.

tclune commented 2 years ago

I'm now assigning this ticket to myself.

tclune commented 2 years ago

So here is the result for say VEGDYN of including the new timers in the legacy calls:

Legacy report

  Times for VEGDYN
TOTAL                   :       0.664       0.767       0.866
GenInitTot              :       0.325       0.325       0.325
--GenInitMine           :       0.325       0.325       0.325
GenRunTot               :       0.338       0.442       0.541
--GenRunMine            :       0.338       0.442       0.541
GenFinalTot             :       0.000       0.000       0.000
--GenFinalMine          :       0.000       0.000       0.000
GenRecordTot            :       0.000       0.000       0.000
--GenRecordMine         :       0.000       0.000       0.000
GenRefreshTot           :       0.000       0.000       0.000
--GenRefreshMine        :       0.000       0.000       0.000

New:

 Time for VEGDYN
                                                               Inclusive        Exclusive    
                                                            ================ ================
Name                                               #-cycles  T (sec)    %     T (sec)    %   
                                                   -------- --------- ------ --------- ------
VEGDYN                                                   11     0.665 100.00     0.000   0.02
--SetService                                              1     0.001   0.15     0.001   0.14
----GenSetService                                         1     0.000   0.01     0.000   0.01
--Initialize                                              1     0.326  48.94     0.000   0.00
----GenInitialize                                         1     0.326  48.94     0.000   0.00
------TOTAL                                               1     0.326  48.93     0.000   0.00
--------GenInitTot                                        1     0.325  48.93     0.000   0.00
----------GenInitMine                                     2     0.325  48.93     0.000   0.00
------------GenInitialize_self                            2     0.325  48.92     0.325  48.92
--Record                                                  4     0.000   0.02     0.000   0.00
----TOTAL                                                 4     0.000   0.02     0.000   0.01
------GenRecordTot                                        4     0.000   0.01     0.000   0.01
--------GenRecordMine                                     4     0.000   0.01     0.000   0.00
----------Record_self                                     4     0.000   0.00     0.000   0.00
--Run                                                     4     0.338  50.83     0.000   0.01
----GenRunTot                                             4     0.338  50.83     0.000   0.01
------GenRunMine                                          4     0.338  50.82     0.000   0.02
--------TOTAL                                             4     0.338  50.81     0.338  50.81
--Finalize                                                1     0.000   0.03     0.000   0.00
----TOTAL                                                 1     0.000   0.03     0.000   0.00
------GenFinalTot                                         1     0.000   0.03     0.000   0.00
--------GenFinalMine                                      1     0.000   0.03     0.000   0.00
----------Final_self                                      1     0.000   0.02     0.000   0.02

I don't think this is what we want. The TimerOn and TimerOff calls in the gridcomps should just be for unusual things. The generic timers already get the usual stuff.

mathomp4 commented 2 years ago

So weird that one of the TOTAL is at a different level. Huh. Might point to a "wrong order of calls" in VEGDYN.