cloudsci / cloudmetrics

Toolkit for computing 15+ metrics characterising 2D cloud patterns
16 stars 8 forks source link

Removed caching from _get_regionprops, and instead compute these always #73

Closed martinjanssens closed 1 year ago

martinjanssens commented 1 year ago

To deal with #70, I've removed the caching from _get_regionprops, since

  1. It wasn't that big of a performance boost anyway
  2. It didn't actually do anything if the object metrics were computed from masks directly, since e.g. cloudmetrics.mask.iorg_objects(da_cloudmask) first does a label_objects(da_mask) anyway, which gives a unique array of object_labels per function call, so _get_regionprops would just make a new cache entry for these instead of reusing the cache
  3. The cache now no longer accumulates memory
  4. We don't build assumptions into the library on the order that people want to compute metrics in (releasing the cache whenever a new scene comes by sort of imposes a workflow where people first loop over scenes, so that scene can be reused)

I've kept the legacy code, in case we want to reintroduce a cache at some point.

leifdenby commented 1 year ago

@martinjanssens remember to update the CHANGELOG https://github.com/cloudsci/cloudmetrics/blob/master/CHANGELOG.md

Should we create a new release once we've got this merged in? Sounds like a change that will be useful for a lot of people. Following semver I think v0.3.0 would make sense since we didn't break anything and it isn't a bug fix, but a "feature" (the feature of running better and faster!)

martinjanssens commented 1 year ago

@leifdenby yes I agree with releasing another version, potentially including Hauke's new version of the open sky function, that seems a lot quicker. Let's discuss in a new issue!