optimizer icon indicating copy to clipboard operation
optimizer copied to clipboard

optimizer_test.py failure on litprotobuf ERROR

Open jiafatom opened this issue 4 years ago • 13 comments

I installed onnx/optimizer from source as mentioned. Then I run python onnxoptimizer/test/optimizer_test.py, I got the failure below:

[libprotobuf ERROR google/protobuf/descriptor_database.cc:644] File already exists in database: onnx/onnx-ml.proto
[libprotobuf FATAL google/protobuf/descriptor.cc:1371] CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size): 
terminate called after throwing an instance of 'google::protobuf::FatalException'
  what():  CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size): 
Aborted (core dumped)

Any suggestions? Thanks!

jiafatom avatar Apr 09 '21 23:04 jiafatom

The underlying issue appears the same as https://github.com/microsoft/onnxruntime/issues/5035. Using protobuf-lite is a working workaround for me:

CMAKE_ARGS="-DONNX_USE_LITE_PROTO=ON" python setup.py build

Other related discussions: https://github.com/onnx/onnx/issues/3030, https://github.com/protocolbuffers/protobuf/issues/1941

yan12125 avatar Apr 10 '21 02:04 yan12125

Thanks for the suggestion @yan12125, that helped!

I'm now getting the below failure

onnxoptimizer/examples/onnx_optimizer_exec.cpp:15:24: error: ‘class onnx::ModelProto’ has no member named ‘ParseFromIstream’; did you mean ‘ParseFromString’?
   bool success = model.ParseFromIstream(&ifs);
                        ^~~~~~~~~~~~~~~~
                        ParseFromString
onnxoptimizer/examples/onnx_optimizer_exec.cpp:25:23: error: ‘const class onnx::ModelProto’ has no member named ‘SerializePartialToOstream’; did you mean ‘SerializePartialToString’?
   success = new_model.SerializePartialToOstream(&ofs);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
                       SerializePartialToString

Looks like a protobuf version issue. I can proceed to build the project and install the whl package if I remove the code associated with ParseFromIstream and SerializePartialToOstream in this file. Perhaps the example script should be modified to allow building under a wider range of protobuf version?

BowenBao avatar Apr 13 '21 01:04 BowenBao

Hmm, ParseFromIstream and SerializePartialToOstream are available from full protobuf but not protobuf-lite before 3.9.0 [1].

Perhaps the example script should be modified to allow building under a wider range of protobuf version?

I think that is a good way. Let's see what @daquexian thinks.

[1] https://github.com/protocolbuffers/protobuf/commit/1d4e9593745c1cbff916cb5ab9c0c87d07369fcb

yan12125 avatar Apr 13 '21 02:04 yan12125

Sorry for the late reply.

@yan12125 @jiafatom @bowenbao Could you please try linking to protobuf static lib? I found that if onnx and onnxoptimizer are both built from source and link to protobuf shared lib, the "File already exists in database: onnx/onnx-ml.proto" error will happen. If protobuf is statically linked instead (i.e. linking libprotobuf.a), the error will not happen.

daquexian avatar Apr 18 '21 13:04 daquexian

Thanks a lot for the hint. I managed to build onnxoptimzer with a static protobuf library and it indeed works. I got some issues during the build, so I'd like to share my steps in hope of helping others:

  1. First uninstall the system protobuf. Seems libprotobuf.so interferes with libprotobuf.a
  2. Download and extract protobuf sources. I use https://github.com/protocolbuffers/protobuf/releases/download/v3.15.8/protobuf-cpp-3.15.8.tar.gz
  3. Configure and build protobuf with CXXFLAGS="-fPIC" ./configure --disable-shared; make; make install DESTDIR=$HOME/protobuf-install
  4. Build onnxoptimizer with PATH="$PATH:$HOME/protobuf-install/usr/local/bin" python setup.py build

I feel the process more complex than using protobuf-lite provided by Linux distributions. Also, I'm maintaining a unofficial python-onnxoptimizer package for Arch Linux, and packaging guidelines there recommend against using static libraries for concerns around security and package sizes - so I use protobuf-lite for that package.

yan12125 avatar Apr 26 '21 13:04 yan12125

@yan12125 wow! I had been an arch linux user until one year ago 😂 (I'm now using ChromeOS/Windows and most of my daily work is to ssh to server) Thanks for your great contribution to archlinuxcn and aur!

But I'm afraid it is a "trick" to use libprotobuf-lite. The reason that it solves the problem is it makes onnx optimizer link to libprotobuf-lite.so while onnx links to another library, libprotobuf.so. Though it works, it relies on the "implementation" of how onnx is packaged, which is fragile.

daquexian avatar Apr 26 '21 15:04 daquexian

Thanks for your kind words! I'm glad that my efforts are also helpful for others :)

Yep, using protobuf-lite is a trick and it is indeed a little fragile. I share my findings in hope of helping others. Building onnxoptimizer against static protobuf does prevent the conflict between onnx-ml.proto in onnxoptimizer and onnx. However, two copies of protobuf in one process may cause issues if those protobuf copies are built from different versions. I believe the long term solution wil be exposing onnx-ml.proto via shared libraries as suggested by a protobuf contributor (https://github.com/protocolbuffers/protobuf/issues/1941#issuecomment-284582895), so that all libraries can share the same version of protobuf and onnx-ml.proto. Some patches are necessary for ONNX to build working shared libraries. I will do cleanups and submit a pull request when I find time.

yan12125 avatar Apr 29 '21 11:04 yan12125

@yan12125 @daquexian got into same issue in 202222..

Why it still an issue, uninstall system protobuf seems broken everything I rely on, any better suggestion here? optimizer become standalone but shared a onnx-ml.proto with onnx, looks like not a good way, why not just using onnx as included lib remove onnx-ml.proto from this repo?

lucasjinreal avatar Feb 13 '22 13:02 lucasjinreal

Why it still an issue, uninstall system protobuf seems broken everything I rely on

You don't need to uninstall system protobuf if your system protobuf is 3.9.0 or newer. Using the command at https://github.com/onnx/optimizer/issues/38#issuecomment-817058821 to build onnxoptimizer should be enough.

why not just using onnx as included lib remove onnx-ml.proto from this repo?

That is exactly what I wanted to do, but that requires non-trivial changes discussed in https://github.com/onnx/onnx/issues/3030 as you already know. I'm not a developer of onnx or onnxoptimizer and I'm OK with the status quo, so I don't put more efforts into this issue.

yan12125 avatar Feb 13 '22 15:02 yan12125

@yan12125 I believe this error will happen then:

https://github.com/onnx/optimizer/issues/38#issuecomment-818363607

lucasjinreal avatar Feb 14 '22 02:02 lucasjinreal

I saw you mentioned M1 Mac in another issue (https://github.com/onnx/onnx/issues/4013). Do you use a package manager like Homebrew or MacPorts? Both provides protobuf 3.19.x [1,2] and CMAKE_ARGS="-DONNX_USE_LITE_PROTO=ON" should be enough. If there are still failures, please share complete logs.

[1] https://formulae.brew.sh/formula/protobuf [2] https://ports.macports.org/port/protobuf3-cpp/

yan12125 avatar Feb 14 '22 04:02 yan12125

I think is not a protobuf lib issue. It's a prootbuf schema conflict issue

lucasjinreal avatar Sep 28 '22 14:09 lucasjinreal

This is still a problem now, in 2024: https://github.com/pyg-team/pytorch_geometric/issues/9220

yurivict avatar Apr 20 '24 18:04 yurivict