parrt / tensor-sensor

The goal of this library is to generate more helpful exception messages for matrix algebra expressions for numpy, pytorch, jax, tensorflow, keras, fastai.
https://github.com/parrt/tensor-sensor
MIT License
794 stars 40 forks source link

remove the hard torch library dependecies #10

Closed noklam closed 4 years ago

noklam commented 4 years ago

9

Get rid of importing torch.Size by using type(v.shape).name instead/

noklam commented 4 years ago

@parrt I try run through the test, it seems working fine for the torch example. The downside of using type(v.shape).name is it doesn't handle the inheritance problem. But I am not aware of any other method to get rid of checking the object without actually importing

parrt commented 4 years ago

Hi @noklam thanks very much for your help! Maybe instead of if isinstance(v.shape, torch.Size), we ask for v.shape.__class__.__name__=='Size' or whatever?

noklam commented 4 years ago

I don't know if there is any difference between type(v.shape).__name__ versus v.shape.__class__.__name__

https://stackoverflow.com/questions/510972/getting-the-class-name-of-an-instance Some suggesting it is a bit different in Python 2, both are fine!

@parrt

parrt commented 4 years ago

Yeah, I guess you're right. I wonder if we need to check if module is torch and name is Shape.

Re inheritance: likely most people are not subclassing torch tensors. if so, though, wouldn't Shape get inherited into subclass and our test would work?

noklam commented 4 years ago

@parrt Yes, I think it should still work. I think you mean name is Size ?

p.s. I know fastai is subclassing torch tensor. :) https://github.com/fastai/fastai/blob/e34fee816f5dcbf05a9bc9b44e617902a52549fc/fastai/torch_core.py#L300

Btw the test1.py is failing

with dbg():
    z = torch.sigmoid(self.Whz @ h + self.Uxz @ x + self.bz)
    b = W @ b + np.abs(x)

I am getting

NameError: name 'self' is not defined
noklam commented 4 years ago

Added the checking for module and name

parrt commented 4 years ago

yeah test?.py are just junk drawers. deleted test1.py

yep, Shape=>Size

parrt commented 4 years ago

Looks great! Merging.

parrt commented 4 years ago

Done! Does it look right? Ter

On Oct 10, 2020, at 11:25 AM, noklam notifications@github.com wrote:

hacktoberfest-accepted

noklam commented 4 years ago

Thanks it works! Can you add it to #3 too?

parrt commented 4 years ago

Done!

On Sat, Oct 10, 2020 at 9:07 PM noklam notifications@github.com wrote:

Thanks it works! Can you add it to #3 https://github.com/parrt/tensor-sensor/pull/3 too?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/parrt/tensor-sensor/pull/10#issuecomment-706646765, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABLUWNOL4IVCMI7AJSG2DLSKEVP7ANCNFSM4SLE25OA .

-- Dictation in use. Please excuse homophones, malapropisms, and nonsense.

noklam commented 4 years ago

@parrt Thanks!