Fix AllSet instantiation with default activation - #378
Conversation
AllSet defaulted mlp_activation to None, which overrode the nn.ReLU default of AllSetLayer and reached the MLP helper, where the activation class was called unconditionally, raising TypeError: 'NoneType' object is not callable on construction. - Default mlp_activation to torch.nn.ReLU in AllSet - Skip None activations in both copy-pasted MLP helpers, mirroring the existing norm_layer guard - Apply the same fixes to AllSetTransformerBlock and its MLP helper, which shared the latent bug Fixes #307
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #378 +/- ##
==========================================
- Coverage 96.43% 96.36% -0.08%
==========================================
Files 58 58
Lines 2078 2092 +14
==========================================
+ Hits 2004 2016 +12
- Misses 74 76 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a construction-time error in the hypergraph AllSet models where mlp_activation=None could propagate into MLP helpers that unconditionally instantiated the activation, causing TypeError: 'NoneType' object is not callable.
Changes:
- Default
mlp_activationtoReLUinAllSetandAllSetTransformerBlockconstructors. - Guard against
activation_layer is Nonein the MLP helper implementations (mirroring the existingnorm_layerguard). - Apply the same activation/default + guard fixes consistently across both the AllSet and AllSetTransformer implementations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| topomodelx/nn/hypergraph/allset.py | Changes AllSet default mlp_activation to torch.nn.ReLU to prevent None overriding downstream defaults. |
| topomodelx/nn/hypergraph/allset_transformer_layer.py | Defaults mlp_activation to nn.ReLU and skips activation construction when None in the local MLP helper. |
| topomodelx/nn/hypergraph/allset_layer.py | Skips activation construction when None in the local MLP helper to avoid instantiation-time errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| layer_dropout=0.2, | ||
| mlp_num_layers=2, | ||
| mlp_activation=None, | ||
| mlp_activation=torch.nn.ReLU, |
There was a problem hiding this comment.
Fixed in commit 8b99f8f by updating the mlp_activation docstring to callable | None with default torch.nn.ReLU.
Co-authored-by: ffl096 <2105496+ffl096@users.noreply.github.com>
AllSet defaulted mlp_activation to None, which overrode the nn.ReLU default of AllSetLayer and reached the MLP helper, where the activation class was called unconditionally, raising TypeError: 'NoneType' object is not callable on construction.
Fixes #307