Merged
Conversation
ff1b6f3 to
e9cd95a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
ef06caf to
6b35069
Compare
* Add ModuleFactory * Add get_in_out_shapes * Add condition on model being a ShapedModule before doing assertion about output shape in _forward_pass * Update module instantiation in tests to use ModuleFactories instead of type[ShapedModule]
6b35069 to
fb19f47
Compare
ValerianRey
added a commit
that referenced
this pull request
Oct 20, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The direct consequence is that we can now test model that have
__init__arguments. These args will be provided to theModuleFactory, and then each instance created will receive these args.Another nice consequence is that we can now test nn.Module that are not ShapedModule, simply by adding a line in get_in_out_shapes to indicate the formula to get the input and output shapes. So for instance, to test a Linear, one would have to add:
in
get_in_out_shapes, and:to the
PARAMETRIZATIONSoftest_engine.py.Note that for example for nn.Conv2d, this would be much more complex, because it depends on all the parameters (maybe there are online code snippets to compute this).
We also don't have to worry about device anymore! The factory imports the
DEVICEfromconftest.py, and directly moves the created models to it (similarly as the tensor creation functions fromtest.utils.tensors.py).Lastly, we don't have to worry about resetting the seed to 0 before creating a model. The factory is in charge of forking the rng and setting the seed to 0 before each model creation. Arguably, we could have a
seed: int | Noneparameter to the__call__method ofModuleFactory, so that we can optionally not do that, but for now this is good enough I think.The downside is that for now, the pytest summary has the name "factory0", "factory1", etc., for the different parametrizations, but this is easy to fix. I'll make another PR to fix this + other parameter name issues with pytest.