Skip to content

Comments

feat(autojac): Make jac_to_grad return optional weights#586

Open
ValerianRey wants to merge 7 commits intomainfrom
make-jac-to-grad-return-weights-2
Open

feat(autojac): Make jac_to_grad return optional weights#586
ValerianRey wants to merge 7 commits intomainfrom
make-jac-to-grad-return-weights-2

Conversation

@ValerianRey
Copy link
Contributor

@ValerianRey ValerianRey commented Feb 20, 2026

This is based on #585 but instead of taking a method argument that can be either Aggregator or Weighting, it always takes an aggregator.

  • feat(autojac): Make jac_to_grad return optional weights
  • Add test with Weighting
  • Make jac_to_grad only take aggregator
  • Make the aggregator parameter positional & keyword

There's just one problem: currently, WeightedAggregator is not public. So we can't really talk about this class in the docstring of jac_to_grad. I think we have two options:

  • Make WeightedAggregator public. Because it has a weighting parameter, of type Weighting[Matrix], we'll have to find a solution for that too (make Matrix public or annotate weighting as Weighting).
  • Say something like "If the aggregator is based on a Weighting" instead of "If a :class:WeightedAggregator <torchjd.aggregation._aggregator_bases.WeightedAggregator> is provided"

A similar issue will arise when we'll add the option to do the gramian-sum optimization. We'll have to talk about GramianWeightedAggregator and make it public (as well as PSDMatrix), or we'll have to say "aggregator uses a gramian-based Weighting`.

Note that this latter problem would also happen to some extent in #585, because Weighting[PSDMatrix] is not public, so we'll also have to say something like if weighting is gramian-based or make PSDMatrix public.

I think the easiest solution for now is just to talk about "aggregator using a Weighting", and later about "aggregator using a gramian-based Weighting" when we add the gramian-sum optimization. (Done in b701c59)

PierreQuinton and others added 4 commits February 20, 2026 10:36
* Change `aggregator: Aggregator` to `method: Aggregator | Weighting` and return type to optional `Tensor`.
* Make `method` positional only.
* Add overloads to rename `method` to `aggregator` or `weighting` and link it to output type.
* Compute the weights if we provide a weighting and return them.
* Update the doc and add a usage example
@ValerianRey ValerianRey added cc: feat Conventional commit type for new features. package: autojac labels Feb 20, 2026
@ValerianRey ValerianRey self-assigned this Feb 20, 2026
@ValerianRey ValerianRey force-pushed the make-jac-to-grad-return-weights-2 branch from 5c79901 to b701c59 Compare February 20, 2026 16:25
- We never use it anywhere else in the library, and I think it's quite redundant here
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: autojac

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants