search

CliMA / ClimaDiagnostics.jl

A framework to define and output observables and statistics from CliMA simulations
Apache License 2.0
9 stars 2 forks source link

Acquire ownership of data returned by compute! #88

Closed Sbozzolo closed 1 month ago

Sbozzolo commented 1 month ago

Prior to this version, ClimaDiagnostics would directly store use the output returned by compute! functions the first time they are called. This leads to problems when the output is a reference to an existing object since multiple diagnostics would modify the same object. Now, ClimaDiagnostics makes a copy of the return object so that it is no longer necessary to do so in the compute! function.

Fixes problem found by @szy21 with radiation

Sbozzolo commented 1 month ago

LGTM. I think that this is more along the lines of how the functional version will look, too, if/when we change over to using LazyBroadcast.jl.

Yes, I agree, and I don't know why I didn't think about this earlier

charleskawczynski commented 1 month ago

Yes, I agree, and I don't know why I didn't think about this earlier

This was kind of my related concern with the deepcopy (I forget which repo/PR it was from).

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.19%. Comparing base (57d2261) to head (19437cd).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #88 +/- ## ======================================= Coverage 85.19% 85.19% ======================================= Files 13 13 Lines 520 520 ======================================= Hits 443 443 Misses 77 77 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.