search

ai2cm / fv3net

explore the FV3 data for parameterization
MIT License
16 stars 3 forks source link

Time averaging of python diagnostics #893

Closed nbren12 closed 3 years ago

nbren12 commented 3 years ago

Requirements

  1. Time average set of variables within a certain interval of time
  2. Rich output time coordinate information.
  3. Can output both time averaged and instantaneous data for any variable.
  4. Configure using the same interface as DiagnosticsFile in backwards compatible way.

Implementation

What is the best approach to average python-based outputs in time? Naturally, this functionality could be implemented in one of many places

  1. The time loop. In principle, time averaging could require detailed knowledge of the time-stepping procedure. Cons: this would require more bespoke code and configuration in the time-stepping classes, coupling between the output time frequency and the averaging code.
  2. The DiagnosticFile. pros: similar to FV3, doesn't pollute the state-modifying code with purely diagnostic codeCons: really fancy time-stepping logic could break the averaging. There are a few methods here: a. Implement as a new "ZarrMonitor" object b. Implement a subclass of DiagnosticFile c. Add a flag to DiagnosticFile. d. Add plugin to DiagnosticFile that allows transforming the input data.

Option 1 cannot satisfy Requirement No 3 easily, therefore we choose to implement it on the DiagnosticFile. The separation of time and state in .observe(time, data) seems like a more appropriate interface here than the ZarrMonitors .store method, so 2a is out. DiagnosticFile takes time as an attribute, but averaging fundamentally changes what time is, so this doesn't seem appropriate.

nbren12 commented 3 years ago

@brianhenn What was the motivation for the DiagnosticFile.set_monitor method? It looks like an implementation of Java's builder design pattern, but one could use keyword arguments to accomplish the same thing. Is it no longer possible for the DiagnosticFile to make it's own ZarrMonitor?

Also DiagnosticFile._name seems like it is only used for the default_diagnostics configurations, and no longer plays an direct role in making a zarr with that name.

nbren12 commented 3 years ago

I think I get it now. I guess you wanted to remove the I/O from the init of the DiagnosticFile so it would be appropriate as a config object?

nbren12 commented 3 years ago

Closed by #907