[dynamo] Introduce get_real_value API to TensorVariable
#87091
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87091
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Failures, 1 PendingAs of commit 19c4384: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Right now, example_value is doing two jobs: - We use it to propagate metadata (e.g. return type, shapes, etc.) throughout the graph - We use it to satisfy queries for the actual value (e.g. torch.cond, `assume_constant_result`) This is further complicated by the fact that we have two modes, one where `example_value` is a fake tensor, and one where it is a real tensor (this is the `fake_tensor_propagation` config flag). This leads to scenarios where we don't support every combination of job + mode, e.g. if `fake_tensor_propagation=False`, `assume_constant_result` is broken. This is made worse by the fact that "fake tensor mode" is the default and is required if you want dynamic shapes to work. So, this PR introduces a `get_real_value` API that just runs the graph up to `node` in order to get a concrete value. This API is orthogonal to `example_value`, so it doesn't care about `fake_tensor_propagation`. When `fake_tensor_propagation=True`: `example_value` is a fake tensor, you must use the `get_real_value` API to get a concrete value. This will be the only configuration in the future. When `fake_tensor_propagation=False`: `example_value` and `get_real_value` will produce the same value. This is redundant but we will be removing this config soon. To support this, I introduce a cache for computed real values, to memoize the work involved if we're asking for real values a lot. I attached this state to `OutputGraph` because it seems to be what historically managed `example_value` lifetimes, but idk.
suo
force-pushed
the
suo/example-value
branch
from
1409f6f to
19c4384
Compare
There was a problem hiding this comment.
stamping based on stamp here
https://github.com/pytorch/torchdynamo/pull/1575/files
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 2 additional jobs have failed, first few of them are: trunk ,trunk / macos-12-py3-arm64 / build Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -f "flaky mac" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
Hey @suo. |
This works now because of #87091, so don't error out anymore. [ghstack-poisoned]
…87895) This works now because of #87091, so don't error out anymore. cc @jansel @mlazos @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @chunyuan-w @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx Pull Request resolved: #87895 Approved by: https://github.com/tugsbayasgalan, https://github.com/voznesenskym
…ytorch#87895) This works now because of pytorch#87091, so don't error out anymore. cc @jansel @mlazos @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @chunyuan-w @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx Pull Request resolved: pytorch#87895 Approved by: https://github.com/tugsbayasgalan, https://github.com/voznesenskym
…ytorch#87895) This works now because of pytorch#87091, so don't error out anymore. cc @jansel @mlazos @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @chunyuan-w @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx Pull Request resolved: pytorch#87895 Approved by: https://github.com/tugsbayasgalan, https://github.com/voznesenskym
3 participants
Right now, example_value is doing two jobs:
throughout the graph
assume_constant_result)This is further complicated by the fact that we have two modes, one
where
example_valueis a fake tensor, and one where it is a realtensor (this is the
fake_tensor_propagationconfig flag).This leads to scenarios where we don't support every combination of
job + mode,
e.g. if
fake_tensor_propagation=False,assume_constant_resultisbroken.
This is made worse by the fact that "fake tensor mode" is the default
and is required if you want dynamic shapes to work.
So, this PR introduces a
get_real_valueAPI that just runs the graphup to
nodein order to get a concrete value. This API is orthogonalto
example_value, so it doesn't care aboutfake_tensor_propagation.When
fake_tensor_propagation=True:example_valueis a fake tensor,you must use the
get_real_valueAPI to get a concrete value. Thiswill
be the only configuration in the future.
When
fake_tensor_propagation=False:example_valueandget_real_valuewill produce the same value. This is redundant but wewill be removing this config soon.
To support this, I introduce a cache for computed real values, to
memoize the work involved if we're asking for real values a lot.
I attached this state to
OutputGraphbecause it seems to be whathistorically managed
example_valuelifetimes, but idk.