Skip to content

feat(autogram): Add kwargs support#443

Merged
ValerianRey merged 14 commits intomainfrom
add-kwargs
Oct 2, 2025
Merged

feat(autogram): Add kwargs support#443
ValerianRey merged 14 commits intomainfrom
add-kwargs

Conversation

@ValerianRey
Copy link
Copy Markdown
Contributor

@ValerianRey ValerianRey commented Oct 1, 2025

This adds support for modules that have some keyword arguments passed to their forward method.
Seems to work on a simple example, but needs more heavy testing.

  • Add kwargs support
  • Add WithModuleWithStringKwarg test
  • Add WithModuleWithHybridPyTreeKwarg test
  • Factorize existing architectures
  • Fix some typos

@ValerianRey ValerianRey added cc: feat Conventional commit type for new features. package: autogram labels Oct 1, 2025
@ValerianRey ValerianRey self-assigned this Oct 1, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

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

@PierreQuinton PierreQuinton changed the base branch from main to fix-args October 2, 2025 08:40
Copy link
Copy Markdown
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.

Not clear to me that we do use a non-empty kwargs in the test. But I trust you checked. LGTM

Base automatically changed from fix-args to main October 2, 2025 17:58
@ValerianRey
Copy link
Copy Markdown
Contributor Author

Not clear to me that we do use a non-empty kwargs in the test. But I trust you checked. LGTM

We do it in WithModuleWithHybridKwargs. Its forward method is:

def forward(self, input: Tensor) -> Tensor:
    return self.with_string_arg(s="two", input=input)

=> The hooked module called with_string_arg receives some keyword arguments s and input. Before this PR, the hook would receive nothing, according to the documentation of register_forward_hook (link):

If with_kwargs is False or not specified, the input contains only the positional arguments given to the module. Keyword arguments won’t be passed to the hooks and only to the forward.

Now, those are correctly passed to the hook.

I'm gonna add a few more architectures to test this more heavily.

@ValerianRey ValerianRey merged commit 6d7d140 into main Oct 2, 2025
31 of 32 checks passed
@ValerianRey ValerianRey deleted the add-kwargs branch October 2, 2025 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cc: feat Conventional commit type for new features. package: autogram

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants