Skip to content

add duplicate constant node pass #2570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

cccclai
Copy link
Contributor

@cccclai cccclai commented Mar 21, 2024

Summary:
This diff is the follow up for #2424

In the case like

    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)

op_b and op_a are in two delegated payload and constant_0 have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become

    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)

The backend can tag the node with no_copy to allert users in cases like constants are too large or etc. In this case, a better approach can be

    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)

Differential Revision: D55113232

Copy link

pytorch-bot bot commented Mar 21, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2570

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 3 Unrelated Failures

As of commit 32b0898 with merge base c8f2d8d (image):

NEW FAILURES - The following jobs have failed:

  • pull / test-llama-runner-linux (fp16, buck2) / linux-job (gh)
    RuntimeError: Tried to register an operator (quantized_decomposed::choose_qparams_per_token(Tensor input, ScalarType dtype) -> (Tensor, Tensor)) with the same name and overload name multiple times. Each overload's schema should only be registered with a single call to def(). Duplicate registration: registered at /dev/null:241. Original registration: registered at /dev/null:241
  • pull / test-llama-runner-linux (fp16, cmake) / linux-job (gh)
    RuntimeError: Command docker exec -t 0c240684e7546854b04cc3a4ff4e1409388431631347331335d5aad9a4381f6d /exec failed with exit code 1

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

  • pull / test-llama-runner-linux (fp32, buck2) / linux-job (gh)
    RuntimeError: Tried to register an operator (quantized_decomposed::choose_qparams_per_token(Tensor input, ScalarType dtype) -> (Tensor, Tensor)) with the same name and overload name multiple times. Each overload's schema should only be registered with a single call to def(). Duplicate registration: registered at /dev/null:241. Original registration: registered at /dev/null:241
  • pull / test-llama-runner-linux (fp32, cmake) / linux-job (gh)
    RuntimeError: Tried to register an operator (quantized_decomposed::choose_qparams_per_token(Tensor input, ScalarType dtype) -> (Tensor, Tensor)) with the same name and overload name multiple times. Each overload's schema should only be registered with a single call to def(). Duplicate registration: registered at /dev/null:241. Original registration: registered at /dev/null:241
  • pull / unittest / macos (buck2) / macos-job (gh)
    RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 5

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 21, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Mar 22, 2024
Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: kirklandsign

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Mar 22, 2024
Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: kirklandsign

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Mar 22, 2024
Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: kirklandsign

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Mar 24, 2024
Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: angelayi

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Mar 24, 2024
Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: angelayi

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

Summary:

This diff is the follow up for pytorch#2424


In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become 
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be 
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: angelayi

Differential Revision: D55113232
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55113232

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7f96f5a.

@cccclai
Copy link
Contributor Author

cccclai commented Apr 9, 2024

@pytorchbot cherry-pick --onto release/0.2

Copy link

pytorch-bot bot commented Apr 9, 2024

❌ 🤖 pytorchbot command failed:

@pytorchbot cherry-pick: error: the following arguments are required: -c/--classification

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Try @pytorchbot --help for more info.

@cccclai
Copy link
Contributor Author

cccclai commented Apr 9, 2024

@pytorchbot cherry-pick --onto release/0.2 -c critical

pytorchbot pushed a commit that referenced this pull request Apr 9, 2024
Summary:
Pull Request resolved: #2570

This diff is the follow up for #2424

In the case like
```
    consant_0 (tag_10) ----> op_b (tag_10)
    |-------------> op_a (tag_11)
```

`op_b` and `op_a` are in two delegated payload and `constant_0` have two options:

In this diff, we're making the default behavior as allowing copying, meaning it will become
```
    consant_0 (tag_10)------------------> op_b (tag_10)
    consant_0_copy (tag_11) -------------> op_a (tag_11)
```

The backend can tag the node with `no_copy` to allert users in cases like constants are too large or etc. In this case, a better approach can be
```
    consant_0 (tag_10) ----> op_b (tag_10)
                                    |-----(output consant_0) --------> op_a (tag_11)
```

Reviewed By: angelayi

Differential Revision: D55113232

fbshipit-source-id: 49559a8cc8225b866a399b4db04ef59fc6860cc1
(cherry picked from commit 7f96f5a)
@pytorchbot
Copy link
Collaborator

Cherry picking #2570

The cherry pick PR is at #2945 and it is recommended to link a critical cherry pick PR with an issue

Details for Dev Infra team Raised by workflow job

@cccclai cccclai mentioned this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants