Thermodynamics.jl
Thermodynamics.jl copied to clipboard
Introduce logger type, remove module methods
This PR removes print_warning
and error_on_non_convergence
, which allow users to toggle how Thermodynamics does error handling. With this PR, those methods are removed, and instead users pass in objects that dispatch this error handling. One advantage of this is that dispatch will result in statically elided paths. The upsides of this is that:
- JET won't complain (although we can currently just overload
print_warning
anderror_on_non_convergence
to silence JET) - this PR could result in less register usage
I've looked at NVTX reports for several jobs, and the constructors (where this feature is relevant) are not really a bottleneck. So, 2) may not actually buy us anything.
Codecov Report
Attention: 13 lines
in your changes are missing coverage. Please review.
Comparison is base (
20de40f
) 92.96% compared to head (3d02b13
) 92.43%. Report is 7 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/relations.jl | 89.32% | 11 Missing :warning: |
src/logger.jl | 83.33% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #158 +/- ##
==========================================
- Coverage 92.96% 92.43% -0.53%
==========================================
Files 10 11 +1
Lines 1152 1164 +12
==========================================
+ Hits 1071 1076 +5
- Misses 81 88 +7
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Is that something I could also use in CloudMicrophysics to print warnings when someone is trying to use the parameterizations outside of the acceptable range of inputs?
I don't want to throw an error. But it would be great to have a warning of some sort
Is that something I could also use in CloudMicrophysics to print warnings when someone is trying to use the parameterizations outside of the acceptable range of inputs?
Yes
I don't want to throw an error. But it would be great to have a warning of some sort
Generally, this can be used as follows:
if logger isa WarnAndErrorLogger
# do stuff that is not gpu compatible
end
That way, users can call different style versions:
-
WarnAndErrorLogger
perhaps GPU compatible -
Verbose
GPU incompatible, but prints lots of info - (maybe)
UnsafeLogger
GPU compatible, prints no warnings and does not error (by us)
I'm not sure I like the name "logger" though, it's not solely for logging. It's kind of like a log level, maybe that's a better name?
@vchuravy this is where I wanted some sort of GPU logger.
I'm not 100% sure of names. I would prefer if it didn't get conflated with the existing Julia logging though.
It seems like we may no longer have a choice in CUDA 5.0: https://buildkite.com/clima/climaatmos-ci/builds/13625#018b0189-731b-4c9d-970f-554d7c5740b1/145-235. Going to move forward with this. We don't need to actually use this, but it will nevertheless help with upstream inference.
@simonbyrne, it seems that this functionality will help us get https://github.com/CliMA/ClimaAtmos.jl/pull/2199 merged in. Unless there are naming objections/suggestions, I'd like to merge this.
Can you please give a brief high-level overview on how this works?
Yes. Although, I keep having back-and-forth thoughts on this.
One downside that I'm remembering is that: The src code (e.g., in ClimaAtmos) will need to be changed if we leverage this feature to silence JET noise. Right now, we can just use TD. print_warning() = false
at the top of the JET script.
Maybe we can put this on hold.