Make VariableVersion::DISABLED the default constructor for VariableVersion. by ailzhang · Pull Request #55572 · pytorch/pytorch

ailzhang · 2021-04-07T23:24:27Z

Stack from ghstack:

Move set_version_counter out of DifferentiableViewMeta constructor. #55653 Move set_version_counter out of DifferentiableViewMeta constructor.
Make VariableVersion::DISABLED the default constructor for VariableVersion. #55572 Make VariableVersion::DISABLED the default constructor for VariableVersion.
Relax some limitations of InferenceMode. #54403 Relax some limitations of InferenceMode.

We used to have VariableVersion default constructor
VariableVersion(uint32_t version=0). But sometimes
we override the version_counter right after it's constructed.
E.g in SavedVariable/TensorImpl.
Thus we should make DISABLED the default constructor and else
where using explicit VariableVersion(uint32_t) constructor.
Note this PR effectively changes SavedVariable constructor (which overrides
version_counter_ inside) to use the DISABLED constructor and we
can see the gains in reduced instruction counts.

// benchmark code
timer = Timer(
    "y = x * x",
    """
    x = torch.rand((3, 3)).requires_grad_()
    """,
    language=Language.PYTHON,
)

 λ ~ python compare.py
No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2'
<torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts
object at 0x7f06c48b3a50>
     7236  lookdict_unicode_nodummy
     2600  torch::autograd::VariableType::(...)
      100  0x0000000017751750
       -5  unlink_chunk.isra.0
     -100  0x000000001773e750
     -402  _int_malloc
    -1600  operator delete(...)
    -1600  c10::intrusive_ptr_target::release_resources()
    -2400  c10::VariableVersion::VersionCounter::~VersionCounter()
    -3600  torch::autograd::SavedVariable::operator=(...)
    -4800  operator new(...)
    -6400  torch::autograd::SavedVariable::SavedVariable(...)
    -7200  torch::autograd::SavedVariable::SavedVariable()
    -8400  free
   -16800  malloc
   -24400  _int_free

Total: -67771

Note there're for other callsites(esp. view related) we just keep it unchanged by
explicitly calling VariableVersion(uint32_t) but we should be
able to optimize those in the followup PRs.

Differential Revision: D27669074

…rsion. We used to have VariableVersion default constructor `VariableVersion(uint32_t int version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t int)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` [ghstack-poisoned]

facebook-github-bot · 2021-04-07T23:24:35Z

💊 CI failures summary and remediations

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

3/3 failures possibly* introduced in this PR
- 1/3 non-scanned failure(s)

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_linux_bionic_rocm3_9_py3_6_build (1/2)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

Apr 09 02:41:59 Error generating file

Apr 09 02:41:59         AT_CUDA_CHECK(cub::DeviceSegmentedReduce::Reduce(
Apr 09 02:41:59                       ^
Apr 09 02:41:59 /var/lib/jenkins/workspace/aten/src/ATen/native/hip/SegmentReduce.hip:90:23: error: use of undeclared identifier 'cub'
Apr 09 02:41:59         AT_CUDA_CHECK(cub::DeviceSegmentedReduce::Reduce(
Apr 09 02:41:59                       ^
Apr 09 02:41:59 /var/lib/jenkins/workspace/aten/src/ATen/native/hip/SegmentReduce.hip:105:23: error: use of undeclared identifier 'cub'
Apr 09 02:41:59         AT_CUDA_CHECK(cub::DeviceSegmentedReduce::Reduce(
Apr 09 02:41:59                       ^
Apr 09 02:41:59 18 errors generated when compiling for gfx900.
Apr 09 02:41:59 CMake Error at torch_hip_generated_SegmentReduce.hip.o.cmake:192 (message):
Apr 09 02:41:59   Error generating file
Apr 09 02:41:59   /var/lib/jenkins/workspace/build/caffe2/CMakeFiles/torch_hip.dir/__/aten/src/ATen/native/hip/./torch_hip_generated_SegmentReduce.hip.o
Apr 09 02:41:59 
Apr 09 02:41:59 
Apr 09 02:41:59 caffe2/CMakeFiles/torch_hip.dir/build.make:1195: recipe for target 'caffe2/CMakeFiles/torch_hip.dir/__/aten/src/ATen/native/hip/torch_hip_generated_SegmentReduce.hip.o' failed
Apr 09 02:41:59 make[2]: *** [caffe2/CMakeFiles/torch_hip.dir/__/aten/src/ATen/native/hip/torch_hip_generated_SegmentReduce.hip.o] Error 1
Apr 09 02:41:59 make[2]: *** Waiting for unfinished jobs....
Apr 09 02:42:05 In file included from /var/lib/jenkins/workspace/aten/src/ATen/native/hip/ForeachUnaryOp.hip:4:
Apr 09 02:42:05 In file included from /var/lib/jenkins/workspace/aten/src/ATen/native/hip/ForeachFunctors.cuh:3:
Apr 09 02:42:05 In file included from /var/lib/jenkins/workspace/aten/src/ATen/native/hip/MultiTensorApply.cuh:6:
Apr 09 02:42:05 In file included from /var/lib/jenkins/workspace/aten/src/ATen/native/hip/Loops.cuh:18:

pytorch_xla_linux_bionic_py3_6_clang9_test (2/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Apr 09 04:51:16 ERROR [0.009s]: TestViewOpsXLA (unittest.loader._FailedTest)

Apr 09 04:51:13 + XLA_EXPERIMENTAL=nonzero:masked_select
Apr 09 04:51:13 + run_test python3 /var/lib/jenkins/workspace/xla/test/../../test/test_view_ops.py -v TestViewOpsXLA
Apr 09 04:51:13 + python3 /var/lib/jenkins/workspace/xla/test/../../test/test_view_ops.py -v TestViewOpsXLA
Apr 09 04:51:16 Test results will be stored in test-reports/python-unittest/.var.lib.jenkins.workspace.xla.test.......test.test_view_ops
Apr 09 04:51:16 
Apr 09 04:51:16 Running tests...
Apr 09 04:51:16 ----------------------------------------------------------------------
Apr 09 04:51:16   TestViewOpsXLA (unittest.loader._FailedTest) ... ERROR (0.009s)
Apr 09 04:51:16 
Apr 09 04:51:16 ======================================================================
Apr 09 04:51:16 ERROR [0.009s]: TestViewOpsXLA (unittest.loader._FailedTest)
Apr 09 04:51:16 ----------------------------------------------------------------------
Apr 09 04:51:16 AttributeError: module '__main__' has no attribute 'TestViewOpsXLA'
Apr 09 04:51:16 
Apr 09 04:51:16 ----------------------------------------------------------------------
Apr 09 04:51:16 Ran 1 test in 0.010s
Apr 09 04:51:16 
Apr 09 04:51:16 FAILED (errors=1)
Apr 09 04:51:16 
Apr 09 04:51:16 Generating XML reports...
Apr 09 04:51:16 Generated XML report: test-reports/python-unittest/.var.lib.jenkins.workspace.xla.test.......test.test_view_ops/TEST-unittest.loader._FailedTest-20210409045116.xml

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.1-py3.6

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 to the (internal) Dr. CI Users group.

…rsion. We used to have VariableVersion default constructor `VariableVersion(uint32_t int version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t int)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` ghstack-source-id: b9535ec Pull Request resolved: #55572

… VariableVersion." We used to have VariableVersion default constructor `VariableVersion(uint32_t int version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t int)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` [ghstack-poisoned]

…rsion. We used to have VariableVersion default constructor `VariableVersion(uint32_t int version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t int)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` ghstack-source-id: b9535ec Pull Request resolved: #55572

… VariableVersion." We used to have VariableVersion default constructor `VariableVersion(uint32_t version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` Note there're for other callsites(esp. view related) we just keep it unchanged by explicitly calling `VariableVersion(uint32_t)` but we should be able to optimize those in the followup PRs. [ghstack-poisoned]

…rsion. We used to have VariableVersion default constructor `VariableVersion(uint32_t version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` Note there're for other callsites(esp. view related) we just keep it unchanged by explicitly calling `VariableVersion(uint32_t)` but we should be able to optimize those in the followup PRs. ghstack-source-id: 006a63b Pull Request resolved: #55572

ezyang

Oh goody. What's the strategy for auditing all the sites? (Pray that CI gets all of them?)

ailzhang · 2021-04-08T04:00:13Z

@ezyang I first deleted the default constructor to make sure I got all the implicit use of default constructor in compilation error and then made DISABLED one default. So ideally this should catch all of them already.

albanD

It looks a bit surprising to spray 0s in this code that doesn't really care that the value is 0...
Should we have a ENABLED arg that we use there?

… VariableVersion." We used to have VariableVersion default constructor `VariableVersion(uint32_t version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` Note there're for other callsites(esp. view related) we just keep it unchanged by explicitly calling `VariableVersion(uint32_t)` but we should be able to optimize those in the followup PRs. [ghstack-poisoned]

facebook-github-bot · 2021-04-09T22:56:45Z

@ailzhang merged this pull request in 7671c15.

…rsion. (pytorch#55572) Summary: Pull Request resolved: pytorch#55572 We used to have VariableVersion default constructor `VariableVersion(uint32_t version=0)`. But sometimes we override the version_counter right after it's constructed. E.g in SavedVariable/TensorImpl. Thus we should make DISABLED the default constructor and else where using explicit `VariableVersion(uint32_t)` constructor. Note this PR effectively changes SavedVariable constructor (which overrides version_counter_ inside) to use the DISABLED constructor and we can see the gains in reduced instruction counts. ``` // benchmark code timer = Timer( "y = x * x", """ x = torch.rand((3, 3)).requires_grad_() """, language=Language.PYTHON, ) λ ~ python compare.py No CUDA runtime is found, using CUDA_HOME='/public/apps/cuda/10.2' <torch.utils.benchmark.utils.valgrind_wrapper.timer_interface.FunctionCounts object at 0x7f06c48b3a50> 7236 lookdict_unicode_nodummy 2600 torch::autograd::VariableType::(...) 100 0x0000000017751750 -5 unlink_chunk.isra.0 -100 0x000000001773e750 -402 _int_malloc -1600 operator delete(...) -1600 c10::intrusive_ptr_target::release_resources() -2400 c10::VariableVersion::VersionCounter::~VersionCounter() -3600 torch::autograd::SavedVariable::operator=(...) -4800 operator new(...) -6400 torch::autograd::SavedVariable::SavedVariable(...) -7200 torch::autograd::SavedVariable::SavedVariable() -8400 free -16800 malloc -24400 _int_free Total: -67771 ``` Note there're for other callsites(esp. view related) we just keep it unchanged by explicitly calling `VariableVersion(uint32_t)` but we should be able to optimize those in the followup PRs. Test Plan: Imported from OSS Reviewed By: navahgar Differential Revision: D27669074 Pulled By: ailzhang fbshipit-source-id: a4deb297cc89142ae8bd683284516c881ddf3c87

ailzhang mentioned this pull request Apr 7, 2021

Relax some limitations of InferenceMode. #54403

Closed

facebook-github-bot added the cla signed label Apr 7, 2021

ailzhang requested review from albanD, bhosmer and ezyang April 7, 2021 23:25

ezyang approved these changes Apr 8, 2021

View reviewed changes

bhosmer approved these changes Apr 8, 2021

View reviewed changes

albanD reviewed Apr 8, 2021

View reviewed changes

ailzhang mentioned this pull request Apr 9, 2021

Move set_version_counter out of DifferentiableViewMeta constructor. #55653

Closed

facebook-github-bot closed this in 7671c15 Apr 9, 2021

facebook-github-bot added the Merged label Apr 9, 2021

facebook-github-bot deleted the gh/ailzhang/60/head branch April 13, 2021 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make VariableVersion::DISABLED the default constructor for VariableVersion.#55572

Make VariableVersion::DISABLED the default constructor for VariableVersion.#55572
ailzhang wants to merge 4 commits intogh/ailzhang/60/basefrom
gh/ailzhang/60/head

ailzhang commented Apr 7, 2021 •

edited

Loading

Uh oh!

facebook-github-bot commented Apr 7, 2021 •

edited

Loading

Uh oh!

ezyang left a comment

Uh oh!

ailzhang commented Apr 8, 2021 •

edited

Loading

Uh oh!

albanD left a comment

Uh oh!

facebook-github-bot commented Apr 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

ailzhang commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 2 new failures recognized by patterns

pytorch_linux_bionic_rocm3_9_py3_6_build (1/2)

pytorch_xla_linux_bionic_py3_6_clang9_test (2/2)

ci.pytorch.org: 1 failed

Uh oh!

ezyang left a comment

Choose a reason for hiding this comment

Uh oh!

ailzhang commented Apr 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Apr 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ailzhang commented Apr 7, 2021 •

edited

Loading

facebook-github-bot commented Apr 7, 2021 •

edited

Loading

ailzhang commented Apr 8, 2021 •

edited

Loading