Skip to content

[Operator Mechanism] Add scatter_nd_add double gradient support#79494

Open
leeleolay wants to merge 1 commit into
PaddlePaddle:developfrom
leeleolay:fix-scatter-nd-add-double-grad
Open

[Operator Mechanism] Add scatter_nd_add double gradient support#79494
leeleolay wants to merge 1 commit into
PaddlePaddle:developfrom
leeleolay:fix-scatter-nd-add-double-grad

Conversation

@leeleolay

@leeleolay leeleolay commented Jul 16, 2026

Copy link
Copy Markdown

PR Category

Operator Mechanism

PR Types

Bug fixes

Description

Fixes #79493

Motivation

Molecular force models compute forces as coordinate gradients of energy and differentiate a force loss again during training. paddle.scatter_nd_add currently raises:

The Op scatter_nd_add_grad doesn't have any grad op.

when the first gradient is requested with create_graph=True.

Changes

  • Register scatter_nd_add_double_grad in backward.yaml.
  • Implement the double gradient as grad_x_grad + scatter_nd(index, grad_updates_grad).
  • Handle either optional incoming second gradient.
  • Add scatter_nd_add_grad to the PIR VJP blacklist because its double gradient is composite-only, consistent with index_put_grad and put_along_axis_grad.
  • Add CPU dygraph coverage for the default create_graph=True path and all three optional-input combinations under eager prim.

Validation

  • Full CPU paddle_python build completed successfully (1495/1495).
  • python -m unittest test_scatter_nd_op.TestScatterNdAddDoubleGrad: 2 tests passed.
  • Full test_scatter_nd_op.py: 29 tests passed, 4 device-specific tests skipped.
  • Pre-commit passed for all four changed files.
  • Duplicate-index second gradients are covered for only grad_x_grad, only grad_updates_grad, and both inputs.

是否引起精度变化

@paddle-bot

paddle-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

你的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.

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

未发现需要阻塞合入的问题。关于 optional double-grad 分支的测试覆盖有一条非阻塞建议,详情见行级评论;Pre Commit 已通过,其余 CI 以最终结果为准。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

Comment thread test/legacy_test/test_scatter_nd_op.py Outdated
Comment on lines +662 to +664
x_grad, updates_grad = paddle.grad(
loss, [x, updates], create_graph=True
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 优先级:P2
处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。

当前用例在第一次 paddle.grad 中同时请求 [x, updates],因此后续 double grad 会同时给 x_gradupdates_grad 传入 cotangent;但本 PR 的 YAML/实现新增了 grad_x_gradgrad_updates_grad 两个 optional 分支,且 #79493 的最小复现只请求 updates 的一阶梯度。建议补一个只请求单个一阶梯度输出的断言,避免 optional 输出映射或空分支回归时漏测。

可参考把计算封装成 helper 后增加类似用例:

updates_grad = paddle.grad(loss, updates, create_graph=True)[0]
updates_double_grad = paddle.grad(updates_grad.sum(), updates)[0]
np.testing.assert_allclose(
    updates_double_grad.numpy(),
    np.array([[4.0, 4.0], [4.0, 4.0], [2.0, 2.0]]),
)

若认为现有 [x, updates] 用例已能覆盖单输出可选分支,请说明依据。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. The test now covers only grad_x_grad, only grad_updates_grad, and both optional inputs with duplicate indices. I also added a separate default-eager assertion for the original create_graph=True reproduction. The composite second-gradient numerical checks run with eager prim enabled, consistent with other composite-only higher-gradient tests. Full CPU build and the complete test_scatter_nd_op.py pass.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已确认当前提交已覆盖原建议:test_create_graph 保留默认 eager 下的 create_graph=True 复现路径,test_double_grad 在 eager prim 下分别覆盖 only grad_x_grad、only grad_updates_grad 和两者同时存在的分支,重复 index 的期望值也对齐。该线程我会标记为已解决。

PaddlePaddle-bot

This comment was marked as outdated.

@leeleolay
leeleolay force-pushed the fix-scatter-nd-add-double-grad branch from c72b9cd to 414b60f Compare July 16, 2026 17:32
@leeleolay leeleolay changed the title Add scatter_nd_add double gradient support [Operator Mechanism] Add scatter_nd_add double gradient support Jul 16, 2026

@risemeup1111 risemeup1111 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已复查当前提交,之前关于 optional 分支测试覆盖的建议已补齐,未发现新的阻塞问题。细节见对应的行级讨论。

Powered by Nyanpasu with gpt-5.5 xhigh, please check the suggestions carefully.

@PaddlePaddle-bot PaddlePaddle-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Paddle-CI-Agent | pr_review | 2026-07-17 01:46:21

📋 Review 摘要

PR 概述:为 scatter_nd_add 注册并实现二阶梯度,补充动态图 create_graph/二阶梯度回归测试
变更范围backward.yaml、composite double backward、PIR VJP 黑名单、legacy 单测
影响面 Tag[Operator Mechanism] [Execute Infrastructure]

问题

未发现阻塞性问题。PR 规范问题在下面章节报,不要在这里重复

历史 Findings 修复情况

Finding 问题 状态
F1 历史指出测试只覆盖两个 optional 同时传入;当前新增 "x"/"updates"/"both" 三个子用例,已分别覆盖 grad_x_gradgrad_updates_grad 单独传入。 ✅ 已修复

📝 PR 规范检查

标题缺少官方 Tag;描述结构符合 checklist §D2 模板。

标题建议(可直接复制):

  • [Operator Mechanism] Add scatter_nd_add double gradient support

总体评价

新增 double grad 公式与现有 scatter_nd_add_grad 的数学关系一致,测试覆盖了 duplicate index 的累加和两个 optional 梯度分支。本轮未拉取 CI,未进行编译/测试运行;完整编译和 CI 仍以流水线为准。

@leeleolay

Copy link
Copy Markdown
Author

/re-run all-failed

@leeleolay

leeleolay commented Jul 16, 2026

Copy link
Copy Markdown
Author

The approval gate now passes the review-bot check and reports two remaining code-owner requirements. Could @wanghuancoder please review the backward.yaml / paddle/phi changes, and could @xiaoguoguo626807 please review the new composite registration?

Validation completed locally: full CPU paddle_python build 1495/1495, generated PIR VJP targets 80/80, complete test_scatter_nd_op.py 29 tests passed (4 skipped), and all pre-commit hooks passed. The new tests cover default eager create_graph=True plus only-x, only-updates, and combined optional double-gradient branches. Additional paddle/phi gate checks on the same build also pass: zero-size trailing dimensions (out, x double grad, and updates double grad all shape [2, 0]) and analytical precision checks for both float32 and float64. BigTensor does not exercise new code here because this PR changes no kernel or index arithmetic; the composite double gradient delegates to the existing scatter_nd_add implementation.

@leeleolay

Copy link
Copy Markdown
Author

CI status note: the only non-successful job, Mac-CPU / Build and test, was cancelled by the workflow's 60-minute job timeout. The macOS build completed successfully, and 357/1952 tests had passed with no test failure when GitHub emitted 'The operation was canceled' exactly at the timeout. Coverage, CI-Windows, CI-Build, and CI-H all passed. Please rerun the Mac job if a green aggregate CI result is required.

@PaddlePaddle-bot

PaddlePaddle-bot commented Jul 16, 2026

Copy link
Copy Markdown

🤖 Paddle-CI-Agent | ci_status_monitor | 2026-07-17 10:46:26 UTC+08:00

CI报告基于以下代码生成(30分钟更新一次):
PR commit: 414b60f | Merge base: f92f172 (branch: develop)


1 Required任务 : 47/49 通过

总执行(rerun次数) 总任务 ✅ 通过 ❌ 失败 ⏳ 运行中 ⏸️ 等待中 跳过
88(4) 84 82 1 0 0 0
任务 错误类型 置信度 日志
Check approval 需要 Approval Job

2 失败详情

🔴 Check approval — 需要 Approval(置信度: 高)

该 Job 需要人工 Approval,完成审批后 CI 才会继续执行。

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.

[Autograd] Missing higher-order gradients block molecular force training

3 participants