Commit Graph

6 Commits

Author SHA1 Message Date
Hao Lu
0927e02a6a [caffe2] Do not run RemoveOpsByType on recurrent networks (#45986)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/45986

Recurrent networks have subnets that are not well supported by `RemoveOpsByType`. Here we exclude recurrent networks by adding the same check as in memonger.

Test Plan:
```
buck test //caffe2/caffe2/fb/predictor:black_box_predictor_test
```

AdIndexer canary for sanity check:
https://www.internalfb.com/intern/ads/canary/430059485214766620

Differential Revision: D24167284

fbshipit-source-id: fa90d1c1f34af334a599d879af09d4c0bf7c27bd
2020-10-07 14:07:52 -07:00
Hao Lu
4f163df41a [caffe2] Special handling of If/AsyncIf op in RemoveOpsByType (#42286)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/42286

One more bug to fix. Operators such as If and AsyncIf need special treatment not just in `onnx::SsaRewrite`, but also in `RemoveOpsByType`. The solution needs two steps:
1) add external inputs/outputs of the subnets of If/AsyncIf op to the inputs/outputs of the op
2) if the inputs/outputs of the If/AsyncIf op need to be renamed as a result, the same inputs/outputs of the subnets need to be renamed as well.

I also added unit tests to cover this corner case.

Test Plan:
```
buck test //caffe2/caffe2/fb/predictor:black_box_predictor_test

mkdir /tmp/models
rm -rf /tmp/$USER/snntest
rm -rf /tmp/snntest
buck run mode/opt admarket/lib/ranking/prediction_replayer/snntest_replayer_test/tools:snntest_replay_test -- --serving_paradigm=USER_AD_PRECOMPUTATION_DSNN
```

Differential Revision: D22834028

fbshipit-source-id: c070707316cac694f452a96e5c80255abf4014bc
2020-07-30 02:02:20 -07:00
Hao Lu
39b4701d31 [caffe2][redo] Reimplement RemoveOpsByType with SSA (#41606)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/41606

The previous diff (D22220798 (59294fbbb9) and D22220797) was recently reverted (D22492356 (28291d3cf8), D22492355) because of a bug associated with the op AsyncIf. The AsyncIf op has net_defs as args and the SSA rewriting didn't take that into account. It has a special path for the op If, but not for AsyncIf. Several changes I made to fix the bug:
1) Add op AsyncIf to the special path for If op in SSA rewriting
2) clear inputs/outputs of the netdefs that are args in If/AsyncIf ops because they're no longer valid
3) revert renamed inputs/outputs in the arg netdefs that are in the external_outputs in the parent netdef

2) and 3) are existing bugs in the `SsaRewrite` function that were just never exposed before.

The algorithm for `RemoveOpsByType` is the same as in my previous diff D22220798 (59294fbbb9). The only new changes in this diff are in `onnx::SsaRewrite` and a few newly added unit tests.

(Note: this ignores all push blocking failures!)

Reviewed By: yinghai

Differential Revision: D22588652

fbshipit-source-id: ebb68ecd1662ea2bae14d4be8f61a75cd8b7e3e6
2020-07-17 16:06:43 -07:00
Hao Lu
28291d3cf8 [caffe2] Revert D22220798 (#41302)
Summary: Pull Request resolved: https://github.com/pytorch/pytorch/pull/41302

Test Plan:
```
buck test //caffe2/caffe2/fb/predictor:black_box_predictor_test
```

Differential Revision: D22492356

fbshipit-source-id: efcbc3c67abda5cb9da47e633804a4800d92f89b
2020-07-11 03:28:29 -07:00
Hao Lu
59294fbbb9 [caffe2] Reimplement RemoveOpsByType with SSA (#40649)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/40649

The original implementation of RemoveOpsByType is pretty buggy and does not remove all instances of the ops that should be removed. It's also quite complicated and hard to modify. I reimplemented it by first converting the graph to its SSA form. The algorithm is quite simple once the graph is in SSA form. It's very similar to constant propagation with a few modifications. The hardest part is to deal with the case of removing an op with the output being an output of the predict net, because that output has to be preserved.

(Note: this ignores all push blocking failures!)

Reviewed By: yinghai, dzhulgakov

Differential Revision: D22220798

fbshipit-source-id: faf6ed5242f1e2f310125d964738c608c6c55c94
2020-07-02 02:45:36 -07:00
Alexander Sidorov
e92506a258 BlackBoxPredictor OSS part N + 1 : strip fb/predictor/Transforms.h dependency (#23350) (#23350)
Summary:
Overal context: open-source BlackBoxPredictor as the entry
point for inference in Caffe2 (thread safe abstraction for Caffe2
inference). This should be used in ThroughputBenchmark for the purpose
of framework comparison
This specific diff:
There should be no harm in moving transformation code to
OSS. On the advantages side we will be able to compare production
Caffe2 setup with PyTorch in the most fair way via
ThroughputBenchmark. This approach avoid any complicated
transformation regirstries. Building those proper would be significant
engineering effort as well as production risk. In the past we had SEVs
related to transforms being turned off due to various refactors. Given
that we don't plan to build any other significant investments into
transformation logic except existing ones (like TVM and Glow), and
those also relate to open-source technologies, I came up to the
conclusion of moving to OSS the whole thing.

Pull Request resolved: https://github.com/pytorch/pytorch/pull/23350
ghstack-source-id: 87121538
Pull Request resolved: https://github.com/pytorch/pytorch/pull/24928

Test Plan: waitforsandcastle

Differential Revision: D16445133

Pulled By: salexspb

fbshipit-source-id: a93106489611dfe427b0f144717bc720d04e47f3
2019-08-22 17:11:00 -07:00