Streaming Filter

[c2xu]

This is a major revision to the original PR (#675) of the streaming band-pass filter.

The filters and their target frequencies are no longer hard-coded. Instead, multiple filters with tidal or arbitrary frequencies as their target frequencies can be turned on. The filter names are specified in MOM_input and must consist of two letters/numbers. If the name of a filter is the same as the name of a tidal constituent, then the corresponding tidal frequency will be used as its target frequency. Otherwise, the user must specify the target frequency. In either case, the target frequency is specified by "FILTER_${FILTER_NAME}_OMEGA".

The restarting capability has also been implemented. Because the filtering is a point-wise operation, all variables are considered as fields, even if they are velocity components.

[c2xu]

Also, while working on this PR, I encountered an issue (#773) related to creating restart files, which seems to be a bug to me.

[codecov]

Codecov Report

Attention: Patch coverage is 4.76190% with 60 lines in your changes missing coverage. Please review.

Project coverage is 37.19%. Comparing base (78a7936) to head (70f8687).
Report is 3 commits behind head on dev/gfdl.

Files with missing lines	Patch %	Lines
...parameterizations/lateral/MOM_streaming_filter.F90	0.00%	50 Missing ⚠️
src/core/MOM_barotropic.F90	23.07%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           dev/gfdl     #772      +/-   ##
============================================
+ Coverage     36.67%   37.19%   +0.51%     
============================================
  Files           278      288      +10     
  Lines         84300    85269     +969     
  Branches      15877    16020     +143     
============================================
+ Hits          30921    31715     +794     
- Misses        47549    47609      +60     
- Partials       5830     5945     +115     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Hallberg-NOAA]

I have examined the code in this PR, and overall it looks sensible. However, I do not see where the filtered velocities are being used, either to change the model state or as output. Is this the first of several steps to implement and use the filters, or is there something that I am missing about this PR?

[c2xu]

I have examined the code in this PR, and overall it looks sensible. However, I do not see where the filtered velocities are being used, either to change the model state or as output. Is this the first of several steps to implement and use the filters, or is there something that I am missing about this PR?

Yes, another PR of frequency-dependent internal wave drag will follow once this is approved and merged. However, I think there is a potential problem with defining the additional dimensions in the restarting files (#773). It would be great if you could take a look at it, @Hallberg-NOAA. I also want to add the restarting capability to the harmonic analysis module, but it might be a good idea to do so after this issue is fixed.

[Hallberg-NOAA]

Thank you for the explanation of how this PR fits into the broader development of diagnostics of the tidal and de-tided velocities, for example. Before accepting this, though, I have added a handful of specific suggestions for added comments that I would have found helpful in making sense of the code more quickly and for some minor code changes for efficiency and parsimoniousness. In each case, I leave the decision about whether to accept these suggestions up to you, and I will be happy to accept this PR once you have had a chance to respond to or accept each of the specific suggestions.

For the future changes building on this PR, I think that it would be worthwhile to consider whether it might be more useful to diagnose the tidal velocities at the end of the predictor step call to btstep() rather than at the start of the corrector step call. This would require some additional refactoring, but I think that it would help to line up the filtered velocities and other fields in time with the unfiltered velocities at the time when we usually diagnose them.

[c2xu]

Thank you for the explanation of how this PR fits into the broader development of diagnostics of the tidal and de-tided velocities, for example. Before accepting this, though, I have added a handful of specific suggestions for added comments that I would have found helpful in making sense of the code more quickly and for some minor code changes for efficiency and parsimoniousness. In each case, I leave the decision about whether to accept these suggestions up to you, and I will be happy to accept this PR once you have had a chance to respond to or accept each of the specific suggestions.

For the future changes building on this PR, I think that it would be worthwhile to consider whether it might be more useful to diagnose the tidal velocities at the end of the predictor step call to btstep() rather than at the start of the corrector step call. This would require some additional refactoring, but I think that it would help to line up the filtered velocities and other fields in time with the unfiltered velocities at the time when we usually diagnose them.

I guess it depends on the application. For de-tiding, placing the Filt_accum() call at the end of btstep(), and only calling it for the correction step, would be the correct way to do it. For frequency-dependent wave drag, it needs to be called at the beginning of btstep(), and for both the prediction and correction step, so that the filtered velocities can be used to calculate the frequency-depending drag later in btstep().

[Hallberg-NOAA]

This is getting closer, but I would still consider the condition for the return from Filt_accum(), as I noted in the updated comment. Please either return if dt <= 0.0 (or the equivalent) or explain why that is not the right thing to do.

[c2xu]

This is getting closer, but I would still consider the condition for the return from Filt_accum(), as I noted in the updated comment. Please either return if dt <= 0.0 (or the equivalent) or explain why that is not the right thing to do.

This has been fixed.

[Hallberg-NOAA]

Thank you very much for this valuable contribution.

[Hallberg-NOAA]

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26127 with the expected changes in entries in the MOM_parameter_doc files.

There are two parameters (STREAMING_FILTER_M2 and STREAMING_FILTER_K1) that are no longer supported and are not going through the usual year-long obsoleting process. However because these were only introduced 4 months ago (on Sept. 10, 2024) as prototype code that does not actually alter solutions or lead to any diagnostic output, I am making the judgement call that there is unlikely to be any damage if they are simply dropped. The new, more general streaming filter is activated with the new runtime parameter USE_FILTER, which if selected enables other subsidiary parameters.