Type classes for lazy resampling

[atbenmurray]

Signed-off-by: Ben Murray [email protected]

Fixes # .

Description

A few sentences describing the changes proposed in this pull request.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[wyli]

Thanks, it looks good to me... cc @ericspod @Nic-Ma @rijobro if you don't have any other concerns I'll merge this one today

[Nic-Ma]

Hi @wyli @atbenmurray ,

I see you 2 experts submitted several PRs today, I was busy with other tasks, will try to review all of them tomorrow.
Please keep it open for 1 more day.

Thanks.

[Nic-Ma]

May I know why use the class name with "XXXType"? LazyTransform may be enough? Like other classes in the codebase.

Thanks.

[atbenmurray]

May I know why use the class name with "XXXType"? LazyTransform may be enough? Like other classes in the codebase.

Thanks.
Hi @Nic-Ma,
LazyTransformType is a pure abstract class that can be used by anyone whose transform is capable of lazy operation. It defines a bit of signature that should be adhered to but no implementation. If we then decide that there is common functionality that we want to put into our own transforms, we can have a LazyTransform that inherits from LazyTransformType that also provides that functionality.
Outside of python, it is generally considered good practice to provide a pure type and implementation that inherits from it. It helps with issues like dependency management, and also allows people to wrap existing types more simply. I don't know why people don't do this in python in general; our lives would be much easier if pytorch provided a Tensor interface that their Tensor implementation inherits, for example.

Initially, I was going to call it ILazyTransform as this is a convention in other languages, but @ericspod requested that I rename it. I could see any of the following working:

1. ILazyTransform -> LazyTransform
2. LazyTransformType -> LazyTransform
3. LazyTransform -> LazyTransformBase

I could do 3. but then we would be changing RandomizableTransform -> RandomizableTransformBase and I think that would cause problems, so I went with 2.

[wyli]

I don't know why people don't do this in python in general; our lives would be much easier if pytorch provided a Tensor interface that their Tensor implementation inherits, for example.

numpy and pytorch defines the __array_function__ protocol instead of defining abstract classes which is in my opinion more flexible duck typing pattern...

• https://pytorch.org/docs/stable/notes/extending.html#extending-torch
• https://numpy.org/neps/nep-0018-array-function-protocol.html

[atbenmurray]

I don't know why people don't do this in python in general; our lives would be much easier if pytorch provided a Tensor interface that their Tensor implementation inherits, for example.

numpy and pytorch defines the __array_function__ protocol instead of defining abstract classes which is in my opinion more flexible duck typing pattern...

• https://pytorch.org/docs/stable/notes/extending.html#extending-torch
• https://numpy.org/neps/nep-0018-array-function-protocol.html

This kind of dispatch method is certainly a useful way of solving issues with operator overloading, but it doesn't solve every problem with subtyping ndarrays or tensors. For example, I would have preferred to solve the multi sampling issue by having metatensor keep an internal data tensor that could be shared between multiple metatensor instances, which would be possible with a TensorType interface to inherit from but not possible as metatensor is a tensor, rather than just appearing to be a tensor (this is IMO a shortcoming of pytorch that they don't provide a pure type signature).

I agree that a pure duck typing approach is quite pythonic, but the big libraries use isinstance a lot to validate their inputs which means that you can't get away with a class that walks and quacks like a tensor but isn't of Tensor type. I think people are generally more used to that than they are that checking whether a method has certain attributes or not.

[Nic-Ma]

Hi @ericspod ,

May I know your opinion about this naming question?
I prefer to any method that is consistent with our existing classes in the codebase as we are already 1.0..

Thanks in advance.

[wyli]

/black

[wyli]

/build

[atbenmurray]

It is saying the blossom-ci is failing, but when I check it, is says that it succeeded, but I suspect part of it is not running

[wyli]

It is saying the blossom-ci is failing, but when I check it, is says that it succeeded, but I suspect part of it is not running

I guess torch 1.13 released a few hours ago changed some of the typing. probably unrelated to this PR...

[wyli]

/build

[atbenmurray]

Nice!