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
771 stars 39 forks source link

Viz: Give user control to suppress bracket nodes in ast visualisation #23

Closed sv2518 closed 3 years ago

sv2518 commented 3 years ago

Adresses https://github.com/parrt/tensor-sensor/issues/22

sv2518 commented 3 years ago

I don't think there are tests for the correctness of the graph visualisation, if I am wrong let me know and point me at the code of one of those tests, so I can extend it to also test for the new feature.

parrt commented 3 years ago

Interesting. I wonder if it's simpler to alter the parser to avoid creating the note in the first place.

parrt commented 3 years ago

Dang. I just tried removing the sub expression (...) during parsing and it works great except for I need those parentheses to properly evaluate the sub expressions. So, it seems we do need some kind of filtering mechanism like you suggested. I think we don't really ever want to show the (...) nodes as they really just change precedence, so that likely simplifies your PR.

parrt commented 3 years ago

I'm still playing around. I have augmented the abstract syntax trees so that they include a pointer to the parser which has a pointer to the original code which can give me access to the original string. That allows me to avoid creating the (...) node. almost there!

parrt commented 3 years ago

Got it working so I'm closing this one. See https://github.com/parrt/tensor-sensor/pull/24