`dask`-based computation not entirely lazy about xeofs HOT 3 CLOSED

I've made a small tweak that eliminates at least one unnecessary computation. Feel free to push to the branch for any improvements regarding this issue.

from xeofs.

This also leads to some serious inefficiencies if we set compute=False, because we end up computing the SVD multiple times. With the current structure, it's unavoidable that we compute during the sign flip and then again when we compute the DataContainer objects. Although I wonder if this step could be moved as some postprocessing layer after whatever our first compute call is. May take some significant restructuring.

DataContainer.compute() is particularly inefficient though in this respect, because the loop will end up computing the SVD for each object we need to compute:
https://github.com/nicrie/xeofs/blob/1f38a5b818a5d5fc55720e1119e6f686aef89168/xeofs/data_container/data_container.py#L29-L36

Easily optimized with something like this, where dask.compute will optimize the task graph for all objects simultaneously:

def compute(self, verbose=False):
        computed_data = {k: v for k, v in self.items() if self._allow_compute[k]}
        if verbose:
            with ProgressBar():
                computed_data = dask.compute(computed_data)[0]
        else:
            computed_data = dask.compute(computed_data)[0]
        for k, v in computed_data.items():
            self[k] = v

from xeofs.

Indeed, off the top of my head, just some additional thoughts:

In the Sanitizer

• make the sanity check optional for those cases where we know/expect to not have any NaNs (cross-ref #83 )
• self.is_valid_sample is computed twice, once in fit and then again in transform. It is sufficient to compute in transform only and then assign it self.is_valid_sample if it does not exist yet

https://github.com/nicrie/xeofs/blob/1f38a5b818a5d5fc55720e1119e6f686aef89168/xeofs/preprocessing/sanitizer.py#L74

https://github.com/nicrie/xeofs/blob/1f38a5b818a5d5fc55720e1119e6f686aef89168/xeofs/preprocessing/sanitizer.py#L95-L98

• use dask.compute() to avoid redundant computations for self.is_valid_feature and self.is_valid_sample not sure if actually feasible since we can compute valid samples only when we know the valid features

I hoped that in many cases the decomposition result would fit into memory (say if you only need 10 modes). In that case compute=True is already not too bad. Apart from the Sanitizer check there shouldn't be any redundant computations.

from xeofs.