Skip to content

Conversation

bwasti
Copy link
Contributor

@bwasti bwasti commented Mar 25, 2020

This commit allows one to use an environment variable to enable the fuser in torch/csrc/jit/tensorexpr/

PYTORCH_TENSOREXPR=1 python benchmark.py

This commit also changes the registration to happen by default, removing the requirement for the python exposed "_jit_register_tensorexpr_fuser"

@bwasti bwasti requested a review from apaszke as a code owner March 25, 2020 00:46
@bwasti bwasti requested a review from ZolotukhinM March 25, 2020 00:46
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Mar 25, 2020
@dr-ci
Copy link

dr-ci bot commented Mar 25, 2020

💊 CircleCI build failures summary and remediations

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


None of the build failures appear to be your fault 💚


  • 2/2 broken upstream at merge base 925cdd5 on Mar 24 from 3:57pm to 5:19pm (3 commits; 50eb1a3 - 36e3c00)

    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 job timed out:

  • pytorch_macos_10_13_py3_test

🚧 1 upstream failure:

These were probably caused by upstream breakages:


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.

This comment has been revised 3 times.

Copy link

@ZolotukhinM ZolotukhinM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we start to always register our pass, we should probably just add it to the graph executor pipeline directly. The existing machinery is only used because it is optional.

if (std::string(enable_c_str) == "0") {
return false;
}
return true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If PYTORCH_TENSOREXPR env variable is set then torch._C._jit_set_texpr_fuser_enabled(False/True) calls will have no effect. Is it a desired behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for me, yea. One can override the program with PYTORCH_TENSOREXPR=0

Right now, for benchmarks, this is really useful because I don't need to inspect internals of various scripts. In the future, I see the python interface as eventually becoming deprecated anyway (once it is always on), and this functioning the same way regardless

namespace jit {

static bool texpr_fuser_enabled = true;
static bool texpr_fuser_enabled_ = false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: why underscore at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous code used texpr_fuser_enabled directly, whereas code should now use the function tensorExprFuserEnabled. the underscore is my attempt at "privatizing" the variable so folks don't use it directly

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@facebook-github-bot
Copy link
Contributor

@bwasti merged this pull request in a3e10d2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants