RandKSpaceSpikeNoised could be more generic

[rijobro]

These lines limit this transform to 2 keys. We could just as easily accept a sequence of intensity ranges, and then rather than creating two instantiations RandKSpaceSpikeNoise (one for image, one for label), we could just create as many instances as defined keys.

[rijobro]

I also don't understand the divergence between KSpaceSpikeNoised and RandKSpaceSpikeNoised on this front.

[rijobro]

@yanielc would you be interested in this?

[yanielc]

@rijobro, sure thing, I can work on it.

[rijobro]

Thanks! I notice that you store the transforms as self.t_img and self.t_label. You could have

self.transforms = {}
for k in self.keys:
    self.transforms[k] = RandKSpaceSpikeNoise(...)

and then in __call__:

        for key, transform in self.key_iterator(d, self.transforms):
            if self._do_transform:
                d[key] = transform(d[key])
        return d

[yanielc]

Thanks! I notice that you store the transforms as self.t_img and self.t_label. You could have

self.transforms = {}
for k in self.keys:
    self.transforms[k] = RandKSpaceSpikeNoise(...)

and then in __call__:

        for key, transform in self.key_iterator(d, self.transforms):
            if self._do_transform:
                d[key] = transform(d[key])
        return d

That's good. I'll also make a super class to host the common Fourier stuff.

I can open a PR next week for this stuff, or earlier if it helps to put it on the board.

[rijobro]

No rush. It's going to create conflicts with the stuff I'm currently working on, but that's OK. To allow fft with both numpy and torch inputs, I've modified the fft and ifft methods to this:

    def _shift_fourier(self, x: DataObjects.Images, n_dims: int, data_type: type) -> DataObjects.Images:
        """
        Applies fourier transform and shifts its output.
        Only the spatial dimensions get transformed.

        Args:
            x (np.ndarray): tensor to fourier transform.
        """
        # argument is dim if torch, else axes
        mod, arg = (torch, "dim") if data_type is torch.Tensor else (np, "axes")
        arg_dict = {arg: tuple(range(-n_dims, 0))}
        out: DataObjects.Images = mod.fft.fftshift(mod.fft.fftn(x, **arg_dict), **arg_dict)  # type: ignore
        return out

    def _inv_shift_fourier(self, k: DataObjects.Images, n_dims: int, data_type: type) -> DataObjects.Images:
        """
        Applies inverse shift and fourier transform. Only the spatial
        dimensions are transformed.
        """
        dims = tuple(range(-n_dims, 0))
        out: DataObjects.Images
        if data_type is torch.Tensor:
            out = torch.fft.ifftn(torch.fft.ifftshift(k, dim=dims), dim=dims, norm="backward")
        else:
            out = np.fft.ifftn(np.fft.ifftshift(k, axes=dims), axes=dims)
        return out.real

[yanielc]

No rush. It's going to create conflicts with the stuff I'm currently working on, but that's OK. To allow fft with both numpy and torch inputs, I've modified the fft and ifft methods to this:

    def _shift_fourier(self, x: DataObjects.Images, n_dims: int, data_type: type) -> DataObjects.Images:
        """
        Applies fourier transform and shifts its output.
        Only the spatial dimensions get transformed.

        Args:
            x (np.ndarray): tensor to fourier transform.
        """
        # argument is dim if torch, else axes
        mod, arg = (torch, "dim") if data_type is torch.Tensor else (np, "axes")
        arg_dict = {arg: tuple(range(-n_dims, 0))}
        out: DataObjects.Images = mod.fft.fftshift(mod.fft.fftn(x, **arg_dict), **arg_dict)  # type: ignore
        return out

    def _inv_shift_fourier(self, k: DataObjects.Images, n_dims: int, data_type: type) -> DataObjects.Images:
        """
        Applies inverse shift and fourier transform. Only the spatial
        dimensions are transformed.
        """
        dims = tuple(range(-n_dims, 0))
        out: DataObjects.Images
        if data_type is torch.Tensor:
            out = torch.fft.ifftn(torch.fft.ifftshift(k, dim=dims), dim=dims, norm="backward")
        else:
            out = np.fft.ifftn(np.fft.ifftshift(k, axes=dims), axes=dims)
        return out.real

Ok, I can wait on this. I was thinking of making all Fourier methods be a pytorch implementation under its own class. But I'll wait on your code.

[rijobro]

It could be a month or two until I'm ready as it's going in this big PR, which will get broken down into bit-sized PRs.

[yanielc]

Cool, then I shouldn't wait, I'll work on the changes you recommended.