Reorg for converters in (FX Converter Refactor [1/N]) #1867

narendasan · 2023-04-28T20:09:01Z

Description

Proposes reorganization of the converters to separate IR from implementation.

Introduces SourceIR enum to inform downstream tasks of any necessary special cases.

~~Note: This PR cannot be merged without child PRs updating uses of common_activation and add_activation to use the new method.~~

Type of change

Please delete options that are not relevant and/or add your own.

Refactor

Checklist:

My code follows the style guidelines of this project (You can use the linters)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas and hacks
I have made corresponding changes to the documentation
I have added tests to verify my fix or my feature
New and existing unit tests pass locally with my changes
I have added the relevant labels to my PR in so that relevant reviewers are notified

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

wushirong

I am assuming PR is planned to land in order, so this PR should land before #1868. The size of #1868 is good, please refer to that as example. This pr is still too large and seems risky.

narendasan · 2023-05-08T20:15:24Z

It is not possible to further reduce the size of the PR without the build breaking (i.e. breaking main on merge) since by moving the convert_activation function a number of references must be updated. This constitute 90% of changes in this PR.

There are 3 major themes in this PR

convert_activation is moved and references are updated
an IR flag is added to prevent logging from being misleading (this change is backwards compatible and if split out will create a PR almost identical to this one)
relu converter

You can move the relu change out but that is a trivial change and will not reduce the size of the PR significantly.

…ementation, example changes with ReLU Signed-off-by: Naren Dasan <naren@narendasan.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

narendasan · 2023-05-20T23:42:47Z

@wushirong Can you review this PR. This is gating release of 1.4

wushirong

LGTM

Signed-off-by: Naren Dasan <naren@narendasan.com>

facebook-github-bot added cla signed fx labels Apr 28, 2023

github-actions bot added component: api [Python] Issues re: Python API component: fx labels Apr 28, 2023

github-actions bot requested a review from yinghai April 28, 2023 20:09

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan requested review from frank-wei and wushirong and removed request for yinghai April 28, 2023 20:10

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan force-pushed the converter_reorg_proto branch from 737b667 to dfd98a5 Compare April 28, 2023 20:13

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan added the Story: ATen Op Support label Apr 28, 2023

narendasan changed the title ~~Reorg for converters in FX [1/N]~~ Reorg for converters in (FX Converter Refactor [2/N]) Apr 28, 2023

github-actions bot requested a review from yinghai April 28, 2023 20:29

narendasan changed the title ~~Reorg for converters in (FX Converter Refactor [2/N])~~ Reorg for converters in (FX Converter Refactor [1/N]) Apr 28, 2023

narendasan force-pushed the converter_reorg_proto branch from dfd98a5 to c438fe2 Compare April 28, 2023 22:52

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan force-pushed the converter_reorg_proto branch from c438fe2 to 9cec91e Compare April 28, 2023 22:55

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan force-pushed the converter_reorg_proto branch from 9cec91e to 9864d96 Compare April 28, 2023 23:28

github-actions bot approved these changes Apr 28, 2023

View reviewed changes

narendasan force-pushed the converter_reorg_proto branch from 9864d96 to 8822b66 Compare May 1, 2023 16:23

github-actions bot approved these changes May 1, 2023

View reviewed changes

narendasan force-pushed the converter_reorg_proto branch from 8822b66 to 1696cd2 Compare May 1, 2023 19:57

github-actions bot approved these changes May 1, 2023

View reviewed changes

narendasan mentioned this pull request May 4, 2023

refactor: Centralizing sigmoid implementation (FX Converter Refactor [2/N]) <Target: converter_reorg_proto> #1868

Merged

7 tasks

wushirong reviewed May 5, 2023

View reviewed changes

refactor: Reorging to reduce code duplication and seperating TRT impl…

12f545c

…ementation, example changes with ReLU Signed-off-by: Naren Dasan <naren@narendasan.com>

gs-olive force-pushed the converter_reorg_proto branch from 1696cd2 to 12f545c Compare May 15, 2023 21:25

github-actions bot approved these changes May 15, 2023

View reviewed changes

wushirong approved these changes May 22, 2023

View reviewed changes

narendasan merged commit b23c980 into main May 23, 2023

narendasan added a commit that referenced this pull request Jun 2, 2023

Reorg for converters in (FX Converter Refactor [1/N]) (#1867)

2e58a9e

Signed-off-by: Naren Dasan <naren@narendasan.com>

narendasan added a commit that referenced this pull request Jun 3, 2023

Reorg for converters in (FX Converter Refactor [1/N]) (#1867)

2c34c8e

Signed-off-by: Naren Dasan <naren@narendasan.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorg for converters in (FX Converter Refactor [1/N]) #1867

Reorg for converters in (FX Converter Refactor [1/N]) #1867

narendasan commented Apr 28, 2023 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

wushirong left a comment

narendasan commented May 8, 2023 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

narendasan commented May 20, 2023

wushirong left a comment

Reorg for converters in (FX Converter Refactor [1/N]) #1867

Reorg for converters in (FX Converter Refactor [1/N]) #1867

Conversation

narendasan commented Apr 28, 2023 • edited Loading

Description

Type of change

Checklist:

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

wushirong left a comment

Choose a reason for hiding this comment

narendasan commented May 8, 2023 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

narendasan commented May 20, 2023

wushirong left a comment

Choose a reason for hiding this comment

narendasan commented Apr 28, 2023 •

edited

Loading

narendasan commented May 8, 2023 •

edited

Loading