Skip to content
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

Changed Makefile to support TF2.6.1. #66

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

NobuoTsukamoto
Copy link

@stakach
Copy link

stakach commented May 7, 2024

@NobuoTsukamoto could you update this for TFv2.16.1 ?

@stakach stakach mentioned this pull request May 7, 2024
@NobuoTsukamoto
Copy link
Author

@stakach
This PR is compatible with TensorFlow v2.16.1.
In my environment, I can build correctly with the combination of TensorFlow v2.16.1.

Are any build errors occurring?

@feranick
Copy link
Contributor

feranick commented May 7, 2024

This should be indeed pulled, as the Makefile in its current form is outdated. That said, when I try to build it, I have the following error, despite having flatbuffers installed in the version recommended (23.5.26):

In file included from ../tensorflow/tensorflow/lite/util.cc:30:
../tensorflow/tensorflow/lite/schema/schema_generated.h:25:41: error: static assertion failed: Non-compatible flatbuffers version included
   25 | static_assert(FLATBUFFERS_VERSION_MAJOR == 23 &&
        |                                                                                   ^

@NobuoTsukamoto
Copy link
Author

Please attach all build logs.
What is reported in the line following the error in question?
Probably the wrong flatbuffers version is being reported.
Please install the correct flatbuffers version.

/tensorflow/tensorflow/lite/schema/schema_generated.h:25:41: note: the comparison reduces to ‘(x == 23)’

Also check the flatc version and installation path.

$ flatc --version
flatc version 23.5.26
$ which flatc
/usr/local/bin/flatc

@stakach
Copy link

stakach commented May 7, 2024

@NobuoTsukamoto

In file included from /home/steve/projects/tensorflow/tensorflow/lite/util.cc:30:
/home/steve/projects/tensorflow/tensorflow/lite/schema/schema_generated.h:25:41: error: static assertion failed: Non-compatible flatbuffers version included
   25 | static_assert(FLATBUFFERS_VERSION_MAJOR == 23 &&
      |                                         ^
/home/steve/projects/tensorflow/tensorflow/lite/schema/schema_generated.h:25:41: note: the comparison reduces to ‘(2 == 23)’
flatc --version
=> flatc version 2.0.8

on the latest version of ubuntu

sudo apt install flatbuffers-compiler
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
flatbuffers-compiler is already the newest version (2.0.8+dfsg1-6build1).
flatbuffers-compiler set to manually installed.

@feranick
Copy link
Contributor

feranick commented May 7, 2024

Yes, flatc is in version 2.0.8 for me as well. Using pip to install the proper python package doesn't change the actual version of flatc.

@stakach
Copy link

stakach commented May 7, 2024

This worked to install it

git clone https://github.com/google/flatbuffers.git
cd flatbuffers
git checkout tags/v23.5.26
cmake -G "Unix Makefiles" \
      -DCMAKE_BUILD_TYPE=Release \
      -DCMAKE_INSTALL_PREFIX=/usr/local \
      -DFLATBUFFERS_BUILD_SHAREDLIB:BOOL=ON
make
sudo make install

although the versioning is just based on release date and not semver so probably compatible with the 24 versions

@feranick
Copy link
Contributor

feranick commented May 7, 2024

TF requires a specific version of flatbuffers depending on the TF release as well. For TF 2.16.1 it's 23.5.26.

From: https://github.com/tensorflow/tensorflow/blob/v2.16.1/tensorflow/lite/schema/schema_generated.h

static_assert(FLATBUFFERS_VERSION_MAJOR == 23 &&
              FLATBUFFERS_VERSION_MINOR == 5 &&
              FLATBUFFERS_VERSION_REVISION == 26,
             "Non-compatible flatbuffers version included");

@feranick
Copy link
Contributor

feranick commented May 7, 2024

In any case, the README.md file should reflect that if someone wants to compile with Makefile, you need a specific version of flatbuffers as indicated (including compilation).

@stakach
Copy link

stakach commented May 7, 2024

the other issue is that manually building flatbuffers only installs a static lib but the libedgetpu build expects a shared lib - is there any way to resolve that?

EDIT: updated my previous comment with an updated build - needed to toggle on the shared lib

@NobuoTsukamoto
Copy link
Author

Please specify the FLATBUFFERS_BUILD_SHAREDLIB option.

https://github.com/google/flatbuffers/blob/v23.5.26/CMakeLists.txt#L28

@stakach
Copy link

stakach commented May 8, 2024

yeah I did that and still seeing

Building libedgetpu.so
/usr/bin/ld.gold: error: cannot find -lflatbuffers
collect2: error: ld returned 1 exit status
make: *** [makefile_build/Makefile:202: libedgetpu] Error 1

even though the output of ldconfig -p | grep flatbuffers is

ldconfig -p | grep flatbuffers
	libflatbuffers.so.23.5.26 (libc6,x86-64) => /usr/local/lib/libflatbuffers.so.23.5.26
	libflatbuffers.so.2 (libc6,x86-64) => /lib/x86_64-linux-gnu/libflatbuffers.so.2
	libflatbuffers.so (libc6,x86-64) => /usr/local/lib/libflatbuffers.so

@NobuoTsukamoto
Copy link
Author

Add the path to LDFLAGS,
e.g.
export LDFLAGS="-L/usr/local/lib"

@stakach
Copy link

stakach commented May 8, 2024

nice, working for me
Are you able to update the README with these steps @NobuoTsukamoto ?

requires flatbuffers v23.5.26

git clone https://github.com/google/flatbuffers.git
cd flatbuffers
git checkout tags/v23.5.26
cmake -G "Unix Makefiles" \
      -DCMAKE_BUILD_TYPE=Release \
      -DFLATBUFFERS_BUILD_SHAREDLIB:BOOL=ON
make
sudo make install

# if libedgetpu build fails
export LDFLAGS="-L/usr/local/lib"

and hopefully we can get this merged

@NobuoTsukamoto
Copy link
Author

Modified README with build instructions targeting TF v2.16.1 and FlatBuffers v23.5.26.

@mwprado
Copy link

mwprado commented Sep 18, 2024

new_makefile.patch.txt
this path will fix the makefile (without bazel) build (tested in fedora 40).

feranick added a commit to feranick/libedgetpu that referenced this pull request Sep 18, 2024
Still refers to ancient versions of TF.

In reference to:
google-coral#66
google-coral#72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants