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 loggers when reading and writing weights #3233

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented Dec 9, 2024

Types of change(s)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (affects only documentation or cleanup)
  • Refactor (no functional changes, no api changes)

Checklist

  • Tested this change with a run of GEOSgcm
  • Ran the Unit Tests (make tests)

Description

In the v12 tracking branch, there are prints when reading and writing weight files. This PR brings them to develop but makes them pFlogger prints.

I'll work with @bena-nasa to make sure they actually work and all.

Related Issue

@mathomp4 mathomp4 added the 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. label Dec 9, 2024
@mathomp4 mathomp4 self-assigned this Dec 9, 2024
@mathomp4
Copy link
Member Author

mathomp4 commented Dec 9, 2024

Okay. Good news, it works:

           MAPL: INFO: Writing weight file: rh_00576x00361_00012x00008_00024x00144_00004x00024_method_03
           MAPL: INFO: Writing weight file: rh_02304x01441_00012x00008_00024x00144_00004x00024_method_03

Now, that makes sense since it's the MAPL logger I used.

But this is activated by Extdata so we have:

...
        EXTDATA: INFO: TR_VEG_FRAC updated R bracket with: ExtData/g5chem/sfc/LAI/veg_fraction_x720_y360_t12_2008.nc at time index   4
        EXTDATA: INFO: TR_regionMask updated L bracket with: ExtData/g5chem/sfc/RADON.region_mask.x540_y361.2001.nc at time index   1
           MAPL: INFO: Writing weight file: rh_00576x00361_00012x00008_00024x00144_00004x00024_method_03
           MAPL: INFO: Writing weight file: rh_02304x01441_00012x00008_00024x00144_00004x00024_method_03
...

It's weird, but I don't feel right assuming the only thing that calls this will be the ExtData code. @bena-nasa might know more.

@tclune Is it good/allowed/okay to make the logger dynamic? That is, we could pass in a string to the call for this and then have the logger be based on the caller? So if say ExtData calls it, we pass in 'EXTDATA' and use that for the:

     lgr => logging%get_logger(logger_string)

?

Or am I just overthinking this?

@tclune
Copy link
Collaborator

tclune commented Dec 9, 2024

One fundamental issue here is that we have competing rationales for determining the local logger. On the one hand we have the component hierarchy and that serves us reasonably well for the gridcomps. (And even there, I'm not 100% convinced that my "long" name approach is good. Maybe we should let the logger controls be flat. Would not be able to debug entire subtrees.) The other is the library layers where the usual logger approach makes sense.

So what do we do when a gridcomp calls a utility layer? We might want to control that from the gridcomp, but that would be difficult. Now I'm going to read the details and provide a more specific response.

@tclune
Copy link
Collaborator

tclune commented Dec 9, 2024

@tclune Is it good/allowed/okay to make the logger dynamic? That is, we could pass in a string to the call for this and then have the logger be based on the caller? So if say ExtData calls it, we pass in 'EXTDATA' and use that for the:

This can be done, but I don't think it is a good idea. It makes the logger too much part of the interface, but we want it to be "behind the scenes".

What we should do is decide for any given module what its logger should be. "MAPL" is a fine placeholder. But maybe "MAPL.base" or "MAPL.regridder" or ... For now I'd not worry. In MAPL3 I hope to have a firmer convention.

@mathomp4 mathomp4 marked this pull request as ready for review December 10, 2024 14:42
@mathomp4 mathomp4 requested a review from a team as a code owner December 10, 2024 14:42
@mathomp4
Copy link
Member Author

@tclune Okay. For now it does seem to work in my testing (as shown above). So I'll undraft

@mathomp4 mathomp4 merged commit 4dbc5e3 into develop Dec 10, 2024
50 checks passed
@mathomp4 mathomp4 deleted the feature/mathomp4/weight-logger branch December 10, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants