Skip to content

Example showing how to register Weighting hook is bugged #492

@ValerianRey

Description

@ValerianRey

https://torchjd.org/stable/examples/monitoring/

The hook should be registered to aggregator.weighting.weighting instead of aggregator.weighting (due to a change in the architecture of aggregators a few months ago).

Also, I think the only reason why aggregators and weightings are nn.Module is to be able to register hooks. So I think we could maybe:

  • Stop making aggregators and weightings be nn.Module
  • Add our own function to register hooks, which would be better documented, accept hooks with a nicer signature, etc
  • This would make it possible to have aggregator.register_weighting_hook(hook) for GramianWeightedAggregator, which would be equivalent to what aggregator.weighting.weighting.register_forward_hook(hook) does today.

Not sure if this is worth the effort but maybe we'd drop a bit of technical debt by having aggregators and weightings not be nn.Modules.

Metadata

Metadata

Assignees

No one assigned

    Labels

    cc: docsConventional commit type for changes to the documentation.package: aggregation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions