Skip to content

refactor(autogram): Use GramianComputers working on modules#453

Merged
ValerianRey merged 34 commits intomainfrom
gramian-accumulator-handles-modules
Oct 14, 2025
Merged

refactor(autogram): Use GramianComputers working on modules#453
ValerianRey merged 34 commits intomainfrom
gramian-accumulator-handles-modules

Conversation

@PierreQuinton
Copy link
Copy Markdown
Contributor

@PierreQuinton PierreQuinton commented Oct 12, 2025

  • Add GramianComputer, JacobianBasedGramianComputer and JacobianBasedGramianComputerWithCrossTerms
  • Rename VJP classes to JacobianComputer classes
  • Move ComputeModuleJacobians to JacobianComputer
  • Make GramianAccumulator only accumulate gramians
  • Use GramianComputer instead of JacobianComputer and create them outside of hook
  • Uniformize some parameter ordering and types
  • Remove test_gramian_accumulator
  • Make InterModuleParamReuse xfail

@PierreQuinton PierreQuinton added cc: refactor Conventional commit type for any refactoring, not user-facing, and not typing or perf improvements package: autogram labels Oct 12, 2025
@PierreQuinton PierreQuinton changed the base branch from main to accumulate-jacobian-returns-jacobians October 12, 2025 12:20
Base automatically changed from accumulate-jacobian-returns-jacobians to main October 12, 2025 22:23
@ValerianRey ValerianRey force-pushed the gramian-accumulator-handles-modules branch from fc8e14d to 41d3b0b Compare October 12, 2025 22:34
@ValerianRey
Copy link
Copy Markdown
Contributor

I think I really like this. It's like we fully commit to a module-based engine with this, which has probably a few small disadvantages (like dropping support for InterModuleParamReuse) but makes things simpler and cleaner IMO. Things like InterModuleParamReuse should only be supported with a Node-based engine probably.

@ValerianRey

This comment was marked as resolved.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/torchjd/autogram/_engine.py 100.00% <100.00%> (ø)
src/torchjd/autogram/_gramian_accumulator.py 100.00% <100.00%> (ø)
src/torchjd/autogram/_gramian_computer.py 100.00% <100.00%> (ø)
src/torchjd/autogram/_jacobian_computer.py 100.00% <100.00%> (ø)
src/torchjd/autogram/_module_hook_manager.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ValerianRey ValerianRey marked this pull request as ready for review October 13, 2025 17:49
@ValerianRey

This comment was marked as resolved.

@PierreQuinton

This comment was marked as resolved.

@PierreQuinton

This comment was marked as resolved.

@ValerianRey ValerianRey changed the title refactor(autogram): Make GramianAccumulator track paths to Modules rather than parameters refactor(autogram): Use GramianComputers working on module level Oct 14, 2025
@ValerianRey ValerianRey changed the title refactor(autogram): Use GramianComputers working on module level refactor(autogram): Use GramianComputers working on modules Oct 14, 2025
@ValerianRey ValerianRey merged commit 5595f5d into main Oct 14, 2025
17 checks passed
@ValerianRey ValerianRey deleted the gramian-accumulator-handles-modules branch October 14, 2025 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cc: refactor Conventional commit type for any refactoring, not user-facing, and not typing or perf improvements package: autogram

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants