py4dgeo icon indicating copy to clipboard operation
py4dgeo copied to clipboard

Setting force = True in RegionGrowingAlgorithmBase.run() deletes smoothed_distances

Open hdaan opened this issue 1 year ago • 3 comments

To delete previous seeds and 4D objects-by-change (4D-OBCs) in the analysis object when running the 4D-OBC algorithm, one can set force to True.

The function then calls the invalidate_results() function, without any parameters. The default then deletes the smoothed_distances: https://github.com/3dgeo-heidelberg/py4dgeo/blob/bd7039430bae76accc06c9a42380a721c596aefd/src/py4dgeo/segmentation.py#L697C1-L697C42

This seems to be unwanted behavior, as one probably wants to use the smoothed_distances when computing 4D-OBCs, even if previous seeds and 4D-OBCs are preferably deleted.

A solution would be to either specify which attributes of the analysis object to delete in the invalidate_results function call, or change the default behavior of the invalidate_results function, to not delete the smoothed_distances.

hdaan avatar Feb 28 '24 12:02 hdaan

I think the preferred way is to change the default parameters of invalidate_results() and would support this, as I agree that in most cases users will want to re-run solely the seed detection and segmentation: def invalidate_results(self, seeds=True, objects=True, smoothed_distances=False) here

If parametrizing the function within the run() method, there would be two different behaviors when the function is called automatically or standalone, which could lead to confusion in usage.

A solution at the moment, without changing the source code, is to call invalidate_results() before run() and setting the parameters as wanted:

analysis.invalidate_results(smoothed_distances=False)
analysis.run()

kathapand avatar Mar 04 '24 07:03 kathapand

I do not have strong feelings here. Also, as using precalculated smoothed distances is purely a performance optimization, I do not see too big issues with backwards compatibility. (Also we are in the wild west that is v0.*)

dokempf avatar Mar 04 '24 09:03 dokempf

@hdaan could you implement the suggested change and make a pull request?

kathapand avatar Mar 04 '24 10:03 kathapand