[WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 #4242

jcwchen · 2022-06-01T22:52:29Z

Description
Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21

After upgrading to Protobuf 21. for building ONNX, ONNX with older Python Protobuf (<=3.20) will behave differently from ONNX with Python Protobuf (4.21): the output order for ONNX ModelProto will be different. To keep consistent behavior, the minimum supported Protobuf versiob would be 4.21 after this upgrade.

Motivation and Context
Closes #4239.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

Signed-off-by: jcwchen <jacky82226@gmail.com>

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

Signed-off-by: jcwchen <jacky82226@gmail.com>

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

…jcw/protobuf4

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

.azure-pipelines/Windows-CI.yml

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

xadupre · 2022-06-21T09:07:36Z

workflow_scripts/protobuf/build_protobuf_unix.sh

 mkdir build_source && cd build_source
-cmake ../cmake -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=$INSTALL_PROTOBUF_PATH -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_POSITION_INDEPENDENT_CODE=ON -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release
+cmake ../cmake -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=$INSTALL_PROTOBUF_PATH -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_POSITION_INDEPENDENT_CODE=ON -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=$BUILD_TYPE


Should we consider Python apps sharing messages with C++ using a native extension (see https://developers.google.com/protocol-buffers/docs/reference/python-generated#sharing-messages and #4274 (comment))?

Thank you Xavier for the reminder! Yes it seems that the option PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp should be added.

Just try to understand more context: so current ONNX does not rely on this option with Protobuf 4.21.0+ (Sharing Messages Between Python and C++), but it would be useful for your PR #4274?

Right now, every C++ function exposes with pybind11 receives an onnx graph as a serialized buffer and not the graph itself. (like check_model, shape_inference). I think this serialization / deserialization) has a significant cost and should be avoided. With the current version, removing any serialization in check_model would make it 2 or 3 times faster.

The documentation of protobuf is not really clear about that mechanism. I wonder if this flag should be used when building protobuf for python as well as the version linked with onnx. Default version of protobuf is:

>>> import google.protobuf.internal.api_implementation as api >>> api.Type() 'upb'

So even if PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp for onnx, it does not change the fact the python version is still using another one. Python sharing messages with C++ won't work.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

xadupre · 2022-07-21T08:23:23Z

requirements.txt

@@ -1,3 +1,3 @@
 numpy >= 1.16.6 # TODO: update once TensorFlow 2.6 is end of life
-protobuf >= 3.12.2, <= 3.20.1
+protobuf


protobuf>=3.12.2 ?

xadupre · 2022-07-21T08:25:00Z

workflow_scripts/protobuf/build_protobuf_win.ps1

 )

 echo "Build protobuf from source on Windows."
-Invoke-WebRequest -Uri https://github.com/protocolbuffers/protobuf/releases/download/v3.16.0/protobuf-cpp-3.16.0.tar.gz -OutFile protobuf.tar.gz -Verbose
+Invoke-WebRequest -Uri https://github.com/protocolbuffers/protobuf/releases/download/v21.1/protobuf-cpp-3.21.1.tar.gz -OutFile protobuf.tar.gz -Verbose


/v21.1/ ?

CarlosRDomin · 2022-08-03T00:28:13Z

This is great! Out of curiosity, is there an ETA to mainline this change? Anything we can do to help expedite it? :)

jcwchen · 2022-08-03T00:39:18Z

This is great! Out of curiosity, is there an ETA to mainline this change? Anything we can do to help expedite it? :)

Good question. I hold on this PR because upstream frameworks like PyTorch and TensorFlow are still using Protobuf 3. It's safer to upgrade ONNX's Protobuf version after they have updated theirs. IIUC, TensorFlow will probably upgrade their Protobuf version in next quarter (reference), but I don't know when PyTorch will do so. Therefore at this moment I don't have concrete ETA that when ONNX will upgrade Protobuf, but ONNX will do so right after both PyTorch and TensorFlow have upgraded their Protobuf.

bruprod · 2022-11-22T15:26:47Z

This is great! Out of curiosity, is there an ETA to mainline this change? Anything we can do to help expedite it? :)

This is great! Out of curiosity, is there an ETA to mainline this change? Anything we can do to help expedite it? :)

Good question. I hold on this PR because upstream frameworks like PyTorch and TensorFlow are still using Protobuf 3. It's safer to upgrade ONNX's Protobuf version after they have updated theirs. IIUC, TensorFlow will probably upgrade their Protobuf version in next quarter (reference), but I don't know when PyTorch will do so. Therefore at this moment I don't have concrete ETA that when ONNX will upgrade Protobuf, but ONNX will do so right after both PyTorch and TensorFlow have upgraded their Protobuf.

Hi :) Is there any update on the timeline for this PR? As far as I have seen TF and Pytorch already closed your linked PRs.

jcwchen · 2022-11-22T18:18:51Z

Hi :) Is there any update on the timeline for this PR? As far as I have seen TF and Pytorch already closed your linked PRs.

Although the linked PRs are closed, IIUC both PyTorch and TF are still using old Protobuf version. Recently ONNX has just upgraded its Protobuf to 3.20.2: #4629 for the upcoming ONNX 1.13 release. So this PR will be considered again for the next ONNX 1.14 release, which won't happen soon.

jcwchen · 2023-03-09T20:47:13Z

To faster resolve merge conflict issue, I have another new PR to achieve this: #4956. Please comment there if anyone has concern or thought. I will close this one. Thanks!

upgrade Protobuf to 3.19.0 to accept Protobuf 4.21

f313bf8

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen added build Issues related to ONNX builds and packages CI pipelines Issues related to the CI pipeline dependencies Pull requests that update a dependency file labels Jun 1, 2022

jcwchen requested a review from a team as a code owner June 1, 2022 22:52

jcwchen marked this pull request as draft June 1, 2022 22:52

jcwchen added the run release CIs Use this label to trigger release tests in CI label Jun 1, 2022

jcwchen closed this Jun 1, 2022

jcwchen reopened this Jun 1, 2022

jcwchen added 2 commits June 1, 2022 16:30

force MSVC_RUNTIME=ON

a5b5e6c

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

force MSVC_RUNTIME=ON

b4e8a23

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen force-pushed the jcw/protobuf4 branch from 2213df2 to b4e8a23 Compare June 2, 2022 14:31

jcwchen added 5 commits June 2, 2022 07:38

use 1

0fb1746

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

try 3.18.0

d316d79

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

static MSVC OFF

17f5c0e

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

use 3.20.1 with MSVC fix

a4d4e91

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

3.20.1 updated protobuf ipynb

91ab669

Signed-off-by: jcwchen <jacky82226@gmail.com>

jcwchen force-pushed the jcw/protobuf4 branch from 1689e59 to 91ab669 Compare June 3, 2022 22:24

jcwchen added 3 commits June 3, 2022 17:05

all 4.21.1

851ecd6

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

all 4.21.1

a45760b

Signed-off-by: jcwchen <jacky82226@gmail.com>

use 4.21.1 as minimum

a2ebb82

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen changed the title ~~[WIP] Upgrade Protobuf to 3.19.0 to accept Protobuf 4.21~~ [WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 Jun 7, 2022

jcwchen added 2 commits June 7, 2022 10:40

Merge branch 'jcw/protobuf4' of https://github.com/jcwchen/onnx into …

c30ddc2

…jcw/protobuf4

add build_type in build protobuf script

cf799aa

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen commented Jun 7, 2022

View reviewed changes

.azure-pipelines/Windows-CI.yml Show resolved Hide resolved

missing comma

0ccf734

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen marked this pull request as ready for review June 7, 2022 18:54

jcwchen added 3 commits June 7, 2022 11:54

Merge branch 'main' into jcw/protobuf4

1d12c02

try mini protobuf version

72ad9f8

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

remove protobuf restriction

bf2d95e

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen force-pushed the jcw/protobuf4 branch from bf2d95e to 1d12c02 Compare June 9, 2022 18:12

xadupre reviewed Jun 21, 2022

View reviewed changes

jcwchen added 2 commits June 22, 2022 14:50

PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp

7c4ee97

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

use 4.21.1 at all

af02c91

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

jcwchen marked this pull request as draft July 20, 2022 22:10

xadupre reviewed Jul 21, 2022

View reviewed changes

xadupre mentioned this pull request Nov 24, 2022

Issue With Protobuf Dependency onnx/sklearn-onnx#869

Closed

jcwchen mentioned this pull request Mar 2, 2023

Upgrade Protobuf to v21.12 to accept Python Protobuf 4.21 #4956

Merged

jcwchen closed this Mar 9, 2023

jcwchen deleted the jcw/protobuf4 branch March 9, 2023 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 #4242

[WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 #4242

jcwchen commented Jun 1, 2022 •

edited

Loading

xadupre Jun 21, 2022

jcwchen Jun 21, 2022

xadupre Jun 22, 2022 •

edited

Loading

xadupre Jun 23, 2022 •

edited

Loading

xadupre Jul 21, 2022

xadupre Jul 21, 2022 •

edited

Loading

CarlosRDomin commented Aug 3, 2022

jcwchen commented Aug 3, 2022

bruprod commented Nov 22, 2022

jcwchen commented Nov 22, 2022

jcwchen commented Mar 9, 2023

[WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 #4242

[WIP] Upgrade Protobuf to v21.1 to accept Python Protobuf 4.21 #4242

Conversation

jcwchen commented Jun 1, 2022 • edited Loading

xadupre Jun 21, 2022

Choose a reason for hiding this comment

jcwchen Jun 21, 2022

Choose a reason for hiding this comment

xadupre Jun 22, 2022 • edited Loading

Choose a reason for hiding this comment

xadupre Jun 23, 2022 • edited Loading

Choose a reason for hiding this comment

xadupre Jul 21, 2022

Choose a reason for hiding this comment

xadupre Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

CarlosRDomin commented Aug 3, 2022

jcwchen commented Aug 3, 2022

bruprod commented Nov 22, 2022

jcwchen commented Nov 22, 2022

jcwchen commented Mar 9, 2023

jcwchen commented Jun 1, 2022 •

edited

Loading

xadupre Jun 22, 2022 •

edited

Loading

xadupre Jun 23, 2022 •

edited

Loading

xadupre Jul 21, 2022 •

edited

Loading