search

microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Remove "decl" field of Var, check arg type for Lam+Def #900

Closed acl33 closed 3 years ago

acl33 commented 3 years ago

decl was previously used only for printing and equality checking.

For printing (__str__), introduce a new function Var.decl_str() that prints out the type. Note this'll need merging with #917 and at that point it'll go into __repr__ instead.

For equality checking, I've added a check to Lam.__eq__ and Def.__eq__ that checks argument types are equal; this gains us back some sanity checking similar to what removal of "decl" loses us.

acl33 commented 3 years ago

There's some duplication of __str__ code that's not ideal but we can probably live with.

An earlier version of the PR changed pystr and wasn't caught by tests (perhaps not suprising that removing type hints from generated python didn't break anything!), now fixed.

My main worry is that it feels we are losing some sanity checking in tests (and some asserts during type propagation). I wonder if it'd help to do another PR first that required Expr's to have equal types in order for the Expr's to be equal.

@awf you say "some comments" but I don't think I see them??

acl33 commented 3 years ago

@awf happy with the arg-type checks?