Skip to content

Conversation

@HydrogenSulfate
Copy link
Contributor

@HydrogenSulfate HydrogenSulfate commented Mar 14, 2024

PR types

Function optimization

PR changes

Others

Description

Pcard-75624

Fix the ambiguity of node execution event, the first segment is for calling GradNodeFunction, should be recognized as XXXGradNode computation cost, the second segment is for potential gradient accumulation in backward queue, should not be count into XXXGradNode, or will misleading users who profile paddle program with nsight or other visualization software.

So this PR use Local_XXXGradNode to represent execution time of XXXGradNode function, Global_XXXGradNode to represent execution time of Local_XXXGradNode plus potential gradient accumulation. Thus, Global_XXXGradNode should always be larger than Local_XXXGradNode and Local_XXXGradNode is more significant for profiling.

To achieve this target, this PR modifies eager_gen.py and several manually XXXnode.cc, and move event creation next to node execution for ignoring node(s) skipped in backward node queue(i.e.
image
).

before:
image

after:

As is dipicted below, Global_MultiplyGradNode include 2 parts: Local_MultiplyGradNode execution(grad_output_tensors = (*node)( node_input_buffer->Buffers(), create_graph, is_general_grad);) and gradient accumulation(node_input_buffers_dict[next_node]->add(edge_rank.first, edge_rank.second, grad_output_tensor, create_graph);)
image

@paddle-bot
Copy link

paddle-bot bot commented Mar 14, 2024

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

plit record event into Global and Local for more accurate profile
@HydrogenSulfate HydrogenSulfate force-pushed the fix_event_range branch 2 times, most recently from 3d481a4 to c05787e Compare March 15, 2024 03:20
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@cxxly cxxly left a comment

Choose a reason for hiding this comment

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

LGTM

@cxxly cxxly merged commit 5e7c7af into PaddlePaddle:develop Mar 18, 2024
@HydrogenSulfate HydrogenSulfate deleted the fix_event_range branch March 18, 2024 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants