[TensorExpr] Extend arithmetic simplifier to work with multi variable expressions (Attempt 2)

[nickgg]

#35127 was landed and reverted because I missed a test fail (oops). I have found and fixed the issue, which was due to zero terms being introduced after the point that filtered them out (usually required NAN/INF, e.g. x / INF => 0).

See #35127 for more info.

[dr-ci]

💊 CircleCI build failures summary and remediations

As of commit e8e2497 (more details on the Dr. CI page):

---

✅ None of the build failures appear to be your fault 💚

---

• 1/1 broken upstream at merge base c672a73 since Mar 27

  Please rebase on the viable/strict branch (expand for instructions)

  If your commit is newer than viable/strict, you can try basing on an older, stable commit:

  git fetch https://github.com/pytorch/pytorch viable/strict
  git rebase --onto FETCH_HEAD $(git merge-base origin/master HEAD)
  

  If your commit is older than viable/strict:

  git fetch https://github.com/pytorch/pytorch viable/strict
  git rebase FETCH_HEAD
  

  Check out the recency history of this "viable master" tracking branch.

---

🚧 1 upstream failure:

These were probably caused by upstream breakages:

• pytorch_linux_backward_compatibility_check_test since Mar 27

---

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 53 times.

[ZolotukhinM]

Approving since it's a fix to an already approved PR.

[facebook-github-bot]

@nickgg has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[nickgg]

OK have rebased back on the various commits, this is the one!

[ZolotukhinM]

Approving as already reviewed. Please fix a couple of places (see inline) before landing. Also, I suggest splitting big PRs into a stack of smaller PRs in future. Thanks!

[ZolotukhinM]

It is generally a bad idea to sort on hash, as we've already discovered - it can make tests brittle and can in future lead to non-deterministic behavior (even if it's not like that now). I suggest following up on that - a better way would be to normalize/canonicalize the expression - i.e. define a strict order how sub-expressions should be placed in the parent expression.

[nickgg]

The hash defines an ordering though, doesn't it. The issues we've been having in this diff is that the hash wasn't consistent across platforms.

I'm a bit resistant to some kind of adhoc ordering because in the case of compound opaque objects I think it can get complicated. Am open to suggestions on this though.

[nickgg]

You're not wrong though, I found a bug in the hash and it changes the order of a bunch of things.

[facebook-github-bot]

@nickgg has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@nickgg merged this pull request in 5b3492d.