[autograd] Extract Node class into node.h#179765
[autograd] Extract Node class into node.h#179765colesbury wants to merge 6 commits intogh/colesbury/4/basefrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/179765
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c5e8a54 with merge base afb9b15 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This comment was marked as resolved.
This comment was marked as resolved.
Move the Node class definition from function.h into a new node.h header. function.h becomes a thin wrapper that includes node.h and provides the free functions (create_gradient_edge, collect_next_edges, etc.) that depend on variable.h. This separation is needed to break the include cycle between function.h and variable.h: function.h includes variable.h, and variable.h needs the complete Node type for upcoming intrusive_ptr conversion. node.h avoids this cycle by not including variable.h or graph_task.h. Authored with Claude. ghstack-source-id: e040d8b Pull-Request: pytorch#179765
Move the Node class definition from function.h into a new node.h header. function.h becomes a thin wrapper that includes node.h and provides the free functions (create_gradient_edge, collect_next_edges, etc.) that depend on variable.h. This separation is needed to break the include cycle between function.h and variable.h: function.h includes variable.h, and variable.h needs the complete Node type for upcoming intrusive_ptr conversion. node.h avoids this cycle by not including variable.h or graph_task.h. Authored with Claude. ghstack-source-id: e040d8b Pull-Request: pytorch#179765
| uint32_t input_nr = input_metadata_.size(); | ||
| auto meta_shape = MetadataShape{std::in_place_type<SymIntSmallVec>, shape}; | ||
| input_metadata_.emplace_back( | ||
| options, meta_shape, is_tensor_subclass, is_nested, grad_dtype); |
There was a problem hiding this comment.
Should meta_shape be std::moved here?
There was a problem hiding this comment.
The motivation for doing this extraction in its own commit is to keep the straightforward refactoring separate from other changes. I don't want to mix in unrelated improvements in this commit.
| return input_nr; | ||
| } | ||
|
|
||
| uint32_t add_input_metadata(const at::Tensor& t) noexcept { |
There was a problem hiding this comment.
Should be moved from value instead of const ref. :)
There was a problem hiding this comment.
I think const ref is appropriate. We are emplacing a InputMetadata constructed from the Tensor, not the Tensor itself.
|
@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: Command Details for Dev Infra teamRaised by workflow job |
|
@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 |
Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.14.0) (oldest at bottom): * #179767 * -> #179766 * #179765 * #179764 Replace std::shared_ptr<Node> with c10::intrusive_ptr<Node> throughout the autograd system. Node now inherits from c10::intrusive_ptr_target instead of std::enable_shared_from_this<Node>. Authored with Claude. Pull Request resolved: #179766 Approved by: https://github.com/albanD ghstack dependencies: #179764, #179765
Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.14.0) (oldest at bottom): * #179767 * -> #179766 * #179765 * #179764 Replace std::shared_ptr<Node> with c10::intrusive_ptr<Node> throughout the autograd system. Node now inherits from c10::intrusive_ptr_target instead of std::enable_shared_from_this<Node>. Authored with Claude. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx kadeng chauhang amjames Lucaskabela jataylo azahed98 xmfan aditvenk [ghstack-poisoned]
Stack from ghstack (oldest at bottom):
Move the Node class definition from function.h into a new node.h
header. function.h becomes a thin wrapper that includes node.h and
provides the free functions (create_gradient_edge, collect_next_edges,
etc.) that depend on variable.h.
This separation is needed to break the include cycle between
function.h and variable.h: function.h includes variable.h, and
variable.h needs the complete Node type for upcoming intrusive_ptr
conversion. node.h avoids this cycle by not including variable.h or
graph_task.h.
Authored with Claude.