Clean up types in tests, in preparation for changing APIs. #3904
Conversation
…s where possible.
|
!test |
| KernelExecutor ke; | ||
| ke.compile(&fusion, aten_inputs); | ||
| ke.compile(&fusion, {t0, t1}); |
There was a problem hiding this comment.
I'd prefer the old style, which would make it easier to change the arguments, whereas in the new code all of the compile, run and validate calls need to be maintained consistently. What is the motivation of this change?
There was a problem hiding this comment.
I'm about to change all the function signatures to be KernelArgumentHolder everywhere. I didn't want to send that PR changing them all simultaneously as it'd be hard to dig out the change of the API relative to the changes of the tests. This will temporarily make our tests more impervious to that type change when it comes.
There was a problem hiding this comment.
I tend to be careful with auto. auto in our codebase slowed down my rampup quite a lot. That said, using it more extensively in tests for repetitive functions like runFusionWithInputs sounds OK to me.
|
@wujingyue I 100% agree, I'm doing it because I'm about to change both input and output types to nvFuser to be consistently KernelArgHolder. Once changed we could revert them back to a consistent type. It's mostly for convenience for this change. |
|
!test |
|
!test |
|
H100 runners are not working at the moment. I didn't see any failures locally on H100, merging. |
In tests auto output type when possible, remove c10::IValue for inputs where possible.