feat: exclude refit sensitive ops from TRT compilation#3159
Conversation
|
|
||
| # Set non-refitable ops as disallowed targets. | ||
| if settings.make_refitable: | ||
| settings.torch_executed_ops.update(REFIT_SENSITIVE_OPS) |
There was a problem hiding this comment.
I think its better to do this through validators than maintaining another list and modifying user settings
There was a problem hiding this comment.
-
Currently, the capability validator doesn't have access to user settings. So, I think making the compilation settings in _compiler.py (compile call) a global variable and passing it here would work. https://github.com/pytorch/TensorRT/blob/main/py/torch_tensorrt/dynamo/conversion/_ConverterRegistry.py#L439
-
The other alternative if we don't want to modify user settings is
CONVERTERS.set_disallowed_targets(settings.REFIT_SENSITIVE_OPS)What would be preferred ?
There was a problem hiding this comment.
Dont see a drawback to let validators see user settings, could be relevant later like if a certain converter only works for one data type
There was a problem hiding this comment.
So it would involve updating the converter registry to take the settings as an arg in addition to the node, and some adjustment in the decorator. But overall I think its the best design since we shouldnt be spreading this info in a bunch of global lists, a converter should be able to tell you what it can and cant do on its own
Dont think we need to make settings global we could make a reference a member of the registry like we do the dynamic shape setting
and then it gets injected into the call to the validatorThere was a problem hiding this comment.
Just an additional arg here:
There was a problem hiding this comment.
Got it. modified it now.
narendasan
left a comment
There was a problem hiding this comment.
LGTM, make sure to mark needs cherrypick and ping @lanluo-nvidia
Description
make_refittableis set toFalseeven after this addition.make_refittable=False. This is because in the future, we wantmake_refitable=Trueby default and hence these tests fail. So, explicitly marking these now will avoid those errors.Fixes https://github.com/pytorch/TensorRT/actions/runs/10637055400/job/29490744653?pr=3131
Type of change
Please delete options that are not relevant and/or add your own.
Checklist: