[SLM] Add unit tests for SLM to Relax exporter #16784
Conversation
Follow-up to apache#16777, add unit tests demonstrating desired behavior.
Prior to this commit, the weights used by `nn.Module` instances were required to be `nn.Parameter` instances. This commit allows the weights to instead be `nn.Tensor` instances, defined in terms of other `nn.Parameter` weights. This allows a model to define both the original weights that would be present in an external checkpoint (e.g. a Pytorch or Safetensors file), and the pre-processing that should be performed on those weights. This is a re-implementation of apache#16757, which was reverted in apache#16777. The re-implementation preserves the handling of dynamic shapes specified as python strings, enabling the test cases that were added in apache#16784.
slyubomirsky
left a comment
slyubomirsky
left a comment
There was a problem hiding this comment.
It's good to have test cases, especially when they explain the intended functionality very well. A tutorial featuring based on these examples (perhaps literally generated from the test cases) might be a good investment of time as well, especially if it can be made more visible.
|
|
||
|
|
||
| def test_custom_module(): | ||
| """A module may be exported from nn.Module to Relax""" |
There was a problem hiding this comment.
Perhaps the comment should differ in soem way from that in the above test.
There was a problem hiding this comment.
Good call, and I've updated the docstring to remove the copy/paste duplicate.
Thank you, and that was in part my goal with the sequence of unit tests. Whenever possible, I prefer test cases that double as mini-tutorials, since then they are less likely to become stale than full tutorials. (Though I agree that it would be beneficial to have a tutorial that follows user-focused flow, rather than the feature-focused flow of these unit tests.) |
MasterJH5574
left a comment
MasterJH5574
left a comment
There was a problem hiding this comment.
Thank you @Lunderberg 🙏
* [SLM] Add unit tests for SLM to Relax exporter Follow-up to apache#16777, add unit tests demonstrating desired behavior. * Updated docstrings based on review comment
3 participants
Follow-up to #16777, add unit tests demonstrating desired behavior.