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

Integrate a C++ package manager in the build process #29

Closed
agatti opened this issue Feb 18, 2024 · 12 comments · Fixed by #37
Closed

Integrate a C++ package manager in the build process #29

agatti opened this issue Feb 18, 2024 · 12 comments · Fixed by #37
Assignees
Labels
enhancement New feature or request

Comments

@agatti
Copy link
Contributor

agatti commented Feb 18, 2024

Is your feature request related to a problem? Please describe.

Whilst I'm (slowly) removing usages of Boost from the project, I've noticed that there are some bits of code that could be either removed in favour of using established libraries, or not shipped along with the project (f.ex: unbundling catch2, using opencv for image processing, etc.)

Describe the solution you'd like

A package manager like vcpkg or conan would help in achieving that, and also simplifying CI setup as all dependencies are brought in (and hopefully cached) from the internet - especially for OSes developers have no local access to. I have a personal preference towards vcpkg as it deeply integrates with cmake and does not need anything special installed (conan needs a full python+pip environment set up). Both package managers in question have pretty much identical available packages for installation so either can in theory handle building Degate.

Describe alternatives you've considered

Things still work as they are right now so this does not apply.

Additional context

N/A

@DorianBDev
Copy link
Member

I agree with you since I'm a long-time user of conan on other projects, this could allow simpler setup, CI and more. However, I also think we should keep the number of dependencies low (e.g., removing boost is a long-identified task), so if we use a package manager we should have strict rules before adding a new dependency. The use cases you gave are actually the only ones I can think of (catch2 + opencv), and having a "simple to install" package manager should also be a goal (although I've never used vcpkg ). Since it wouldn't be that long/hard to migrate to vcpkg or conan, I think we can consider it along boost removal. But before we start anything, I'd like to compare vcpkg and conan for our use case, and maybe plan an improvement to CMakeLists.txt.

@DorianBDev
Copy link
Member

For opencv, we should take time to think before switching since new possibilities emerged in the recent years (deep learning), and keeping current algorithms (even if using a lib) could be a waste of time. What I don't know is if we should scale Degate with regard to very large die (work started with "Attached" project type, not finished at all) before improving the image matching (where "classic" algorithms could be unable to handle very large images).

@agatti
Copy link
Contributor Author

agatti commented Feb 20, 2024

Well, I literally have zero experience on deep learning/AI but I've worked a bit with OpenCV in the past...

The advantage that OpenCV could bring compared to the present situation is that it already has support for several kinds of optimisation/hardware acceleration when available on the system it is built/run on, plus I expect their image processing algorithms to be as close to optimal as they can be. I used to deal with single channel images that were up to 4096x4096 in size and it was quite fast.

However, I just checked the OpenCV site (haven't used it in a while) and I found out it does provide some deep learning functionality and the like (see https://docs.opencv.org/4.9.0/d6/d0f/group__dnn.html and https://docs.opencv.org/4.9.0/dd/ded/group__ml.html). Granted, I expect it not being as full featured as a dedicated DL/ML library, but if you can address two very different issues with just one maintained library and then scale up when needed, it doesn't sound that bad to me.

Regarding vcpkg/conan, I agree there are a few things that can be done on the CMake side first. Something I can send a quick PR for is to enable usage of ccache (https://ccache.dev/) when available, but there are bigger issues like the project not being buildable with ninja for example. Should CMake issues be filed as bugs then?

$ cmake .. -GNinja
-- The C compiler identification is GNU 13.2.1
-- The CXX compiler identification is GNU 13.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Using compiler: /usr/bin/c++
-- Boost static libs: ON
-- Boost multithreaded libs: ON
-- Found Boost: /usr/lib/cmake/Boost-1.83.0/BoostConfig.cmake (found version "1.83.0") found components: filesystem system thread 
-- Found Threads: TRUE  
-- Performing Test HAVE_STDATOMIC
-- Performing Test HAVE_STDATOMIC - Success
-- Found WrapAtomic: TRUE  
-- Found OpenGL: /usr/lib/libOpenGL.so   
-- Found WrapOpenGL: TRUE  
-- Found XKB: /usr/lib/libxkbcommon.so (found suitable version "1.6.0", minimum required is "0.5.0") 
-- Found WrapVulkanHeaders: /usr/include  
-- 64 bits configuration
-- Degate version: 2.1.0-beta.1
-- Release date: 2023-10-01
-- Doxygen found
-- Configuring done (5.2s)
-- Generating done (0.1s)
CMake Error:
  Running

   '/usr/bin/ninja' '-C' '/home/agatti/src/Degate/build' '-t' 'recompact'

  failed with:

   ninja: error: build.ninja:1642: multiple rules generate languages/degate_en.ts

  



CMake Generate step failed.  Build files cannot be regenerated correctly.

@DorianBDev
Copy link
Member

I agree for OpenCV, we might have nicer results and performances on a wide range of hardware. However we'll have to build an algorithm to choose what to give to OpenCV, since some Degate users use images of millions of pixels in width and height (so billions of pixels). At the moment we have code that will "zoom out" the analyzed layer and try pattern matching on lower-quality images to limit performance hit. Another solution could be to extract import features from an image (e.g., edges) and match on that (no pixels matching). This is close (or at least close to the idea) of the currently implemented algorithm. Still, I don't know in which order we should take the issue: reimplement better pattern matching using OpenCV first, or handle very large images to extract constraints before attacking pattern matching. Ultimately, I'm sold on using OpenCV, especially if it also supports deep learning stuff (which we could make optional in preferences and use for purposes other than pattern matching).

For CMake I agree, we could try to support ccache, a PR would be welcomed. For ninja, I think it might not be that complicated to support, the issue seems to come from qt language files (maybe we don't generate them correctly).

@DorianBDev
Copy link
Member

After some comparison we can go with vcpkg, seems to be a better fit for Degate.

@agatti
Copy link
Contributor Author

agatti commented Mar 20, 2024

Right - I'll put the de-boost work aside and give the integration a shot.

@agatti
Copy link
Contributor Author

agatti commented Mar 22, 2024

I've had a quick go at it and came up with this: agatti@97b3973

To try it, make sure the vcpkg submodule is present and then in the source root:

unset VCPKG_ROOT
export VCPKG_ROOT="$PWD/vcpkg"
export PATH=$VCPKG_ROOT:$PATH
# Optional, if you want to keep the binary dependencies' artefacts
export VCPKG_BINARY_SOURCES="clear;files,$PWD/binary_cache,read;files,$PWD/binary_cache,readwrite"
# To build application and tests
cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=$PWD/vcpkg/scripts/buildsystems/vcpkg.cmake -GUnix\ Makefiles -DVCPKG_MANIFEST_FEATURES=test -DBUILD_TESTS=ON
# To just build the application
cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=$PWD/vcpkg/scripts/buildsystems/vcpkg.cmake -GUnix\ Makefiles
# And then build as usual
cmake --build build

Vcpkg suggests setting up presets (https://cmake.org/cmake/help/latest/manual/cmake-presets.7.html) to shorten the commandlines, but I haven't done so - this was just to see how hard would it be. Something interesting could be using a set of build scripts based on https://github.com/friendlyanon/cmake-init - as it already has provisions for CI and code checks.

@DorianBDev
Copy link
Member

Nice! Maybe we could start a PR for the vcpkg migration?

@DorianBDev
Copy link
Member

I have a talk to prepare for Degate at FSiC 2024, but when I have some time I'll try to experiment with vcpkg and improve the use of cmake (and maybe try adding basic opencv support).

@DorianBDev
Copy link
Member

Hi @agatti, could you create a PR for agatti/Degate@97b3973? This would allow me to continue this work on an appropriate branch (by giving me right to contribute on that PR), but also let you contribute if you will

@agatti
Copy link
Contributor Author

agatti commented Sep 2, 2024

Done. You may also want to update the submodule to the latest stable vcpkg tag and see if it still builds. My suggestion to use https://github.com/friendlyanon/cmake-init as a base and then merge cmake changes to it still stands, it does all the heavy lifting in setting up the cmake profiles too!

@DorianBDev
Copy link
Member

Perfect, thanks!

@DorianBDev DorianBDev linked a pull request Sep 17, 2024 that will close this issue
7 tasks
@DorianBDev DorianBDev added the enhancement New feature or request label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants