Skip to content

feat(autogram): Do not consider cross-terms #468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(autogram): Do not consider cross-terms #468

wants to merge 7 commits into from
+14 −51

Conversation

@ValerianRey
Copy link
Contributor

@ValerianRey ValerianRey commented Oct 20, 2025 •
edited
Loading

  • Add and use JacobianBasedGramianComputerWithoutCrossTerms
  • Make ModelAlsoUsingSubmoduleParamsDirectly and InterModuleParamReuse xpass
  • Fix missing type hint
  • Test against no cross-terms

@ValerianRey ValerianRey added feat New feature or request package: autogram labels Oct 20, 2025
@ValerianRey ValerianRey self-assigned this Oct 20, 2025
@ValerianRey ValerianRey added feat New feature or request package: autogram labels Oct 20, 2025
@codecov
Copy link

codecov bot commented Oct 20, 2025 •
edited
Loading

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%> (+100.00%) ⬆️
src/torchjd/autogram/_gramian_computer.py 100.00% <100.00%> (+100.00%) ⬆️
src/torchjd/autogram/_module_hook_manager.py 100.00% <100.00%> (+100.00%) ⬆️

... and 49 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ValerianRey
Copy link
Contributor Author

@PierreQuinton That's a pretty good pre-requesite to #458

@ValerianRey ValerianRey force-pushed the do-not-consider-cross-terms branch from 7b0de8d to 6e4c458 Compare October 20, 2025 15:56
PierreQuinton
PierreQuinton reviewed Oct 21, 2025
View reviewed changes
Copy link
Contributor

@PierreQuinton PierreQuinton left a comment

Choose a reason for hiding this comment

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

This deserves experimentation if possible. At least a little bit.

ValerianRey and others added 2 commits October 23, 2025 18:19
@ValerianRey
Simplify even more:
949a84e 
* Remove unused methods (reset and track_forward_call) from GramianComputer and stop calling them
* Make them always return a Tensor (not optionally)
@ValerianRey
Merge branch 'main' into do-not-consider-cross-terms
51bb4b9
@ValerianRey ValerianRey mentioned this pull request Oct 29, 2025
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PierreQuinton PierreQuinton PierreQuinton left review comments

Assignees

@ValerianRey ValerianRey

Labels

feat New feature or request package: autogram

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ValerianRey @PierreQuinton