Skip to content

Conversation

crcrpar
Copy link
Collaborator

@crcrpar crcrpar commented Aug 28, 2025

What does this PR do?

  • append custom_op_ex to the default executor list when thunder.torch.custom_op._register_custom_op is used
  • add register_executor(custom_op_ex) to thunder/executors/custom_op_ex.py
  • add _register_nvfuser_translator that lets us register nvfuser translator rule for an already registered torch.library.custom_op

close #2605

cc @Borda @lantiga

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 28, 2025
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch 3 times, most recently from c772160 to e84169d Compare September 9, 2025 08:05
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from 86ab3da to 6de2392 Compare September 18, 2025 08:35
@crcrpar
Copy link
Collaborator Author

crcrpar commented Sep 18, 2025

the failure seems caused by the "polluted" translation_map & torch->symbol map

@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from 6de2392 to d6a6b29 Compare September 25, 2025 06:26
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from d6a6b29 to c3e7eb3 Compare September 26, 2025 14:02
@crcrpar crcrpar marked this pull request as ready for review September 26, 2025 14:02
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from c3e7eb3 to 72ba46c Compare September 29, 2025 07:47
@crcrpar crcrpar changed the title Implement thunder.executors.custom_op_ex._override_custom_op_forward [torch.library.custom_op] Add _register_nvfuser_translator Sep 29, 2025
@crcrpar crcrpar mentioned this pull request Oct 2, 2025
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from d4a59a4 to 6ce7b86 Compare October 9, 2025 08:31
which allows overriding forward of `torch.library.custom_op`'s forward

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
…ranslator`

Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
Signed-off-by: Masaki Kozuki <mkozuki@nvidia.com>
@crcrpar crcrpar force-pushed the custom_op/override-fwd-def branch from 3e0fa38 to 0f88252 Compare October 12, 2025 13:26
):
if any(sub_bsym.sym.id == _symbol.id for sub_bsym in bsym.subsymbols):
nvfuser_def_for_custom_op_found = True
assert nvfuser_def_for_custom_op_found
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can assert the number of custom op we expect to find in fuser definiton and backward trace

Comment on lines +531 to +534
# Register the custom_op of `mul` with :func:`thunder.torch._register_custom_op`
_symbol = _register_custom_op(mul)
# Register custom nvfuser definition for the already registered custom_op of mul
_register_nvfuser_translator(_symbol, mul_translator)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the final API usage proposal (symbol creation and translation registration)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

from thunder.executors.torchex import _always_executable

register_supported(symbol, translator_for_nvfuser, checker or _always_executable)
register_supported(symbol.id, translator_for_nvfuser, checker or _always_executable)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify why both calls are needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for most cases the first one should be enough. I don't quite remember the exact reason why I have these two here.

Copy link
Collaborator

@mattteochen mattteochen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Only some curiosity from my side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a helper function to add nvfuser translator rule for already registered custom_op

2 participants