Clear ALL_DIAGNOSTICS in define_diagnostics
#977
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit makes the `define_diagnostics` function clear `ALL_DIAGNOSTICS`. The dictionary is only cleared when it has data, and a warning is given. This prevents each `add_diagnostic_variable` call in `define_diagnostics` from throwing overwrite warnings. This only changes the resulting diagnostics when a user calls `add_diagnostic_variable!` and then calls `define_diagnostics!` or `default_diagnostics`. This desired result could still be achieved by swapping the orders of the call.
imreddyTeja
force-pushed
the
tr/restart-diagnostics-warnings
branch
from
d8a3f69 to
e5d00f4
Compare
AlexisRenchon
approved these changes
Jan 14, 2025
|
I am not sure if this is the best solution. Users could add their own diagnostics in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Closes #955
When a simulation is re-run by re-including the experiment script, many diagnostic overwrite warnings are logged. This PR proposes a fix where only a single warning is logged.
The many warnings occur when
default_diagnosticsis called.default_diagnosticscallsdefine_diagnostics!, which repeatedly callsadd_diagnostic_variable, which throws a warning.One idea is to not overwrite and warn when the DiagnosticVariable being overwritten has not changed. This cannot be done because the
compute!fields cannot be checked for equivalence as they are anonymous functions. Facilitating this would require changes to ClimaDiagnostics that allowcompute!to not be an anonymous function.Another option I considered is making
ALL_DIAGNOSTICSnon global, and make a new one whendefault_diagnosticsis called. This would change the interface and functionality, so I decided against it.To-do
Content
This commit makes the
define_diagnosticsfunction clearALL_DIAGNOSTICS. The dictionary is only cleared when it has data, and a warning is given. This prevents eachadd_diagnostic_variablecall indefine_diagnosticsfrom logging overwrite warnings.This only changes the resulting diagnostics when a user calls
add_diagnostic_variable!and then callsdefine_diagnostics!ordefault_diagnostics. This desired result could still be achieved by swapping the orders of the call.