Skip to content
This repository was archived by the owner on Aug 1, 2025. It is now read-only.

Conversation

@suo
Copy link
Member

@suo suo commented Oct 10, 2022

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 suo changed the title example value [dynamo] Introduce get_real_value API to TensorVariable Oct 10, 2022
@suo suo requested a review from voznesenskym October 10, 2022 19:38
@suo
Copy link
Member Author

suo commented Oct 10, 2022

@voznesenskym I wanted to do the thing where I got rid of stashing example_value on the node.meta, but it turned out to be not clean in a variety of ways. So I'll defer it to a future PR.

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.
@jansel
Copy link
Contributor

jansel commented Oct 15, 2022

We have migrated torchdynamo to torch._dynamo and will use the pytorch/pytorch repo for future development. Please resubmit this PR to https://github.com/pytorch/pytorch/

More details and instructions to port this PR over can be found in #1588

@jansel jansel closed this Oct 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants