Add core dependencies installation script #80
Conversation
…d numpy for compatibility
ovysotska
requested review from
omiksik,
pablospe and
mihaidusmanu
as code owners
|
@microsoft-github-policy-service agree |
|
Thanks! There are some scripts in the If you look at the Dockerfile, you'll notice it is similar to your script. The complexity arises because we have different stages: common, scantools, lamar, etc. I also aimed to avoid including development-only dependencies to prevent creating large images. One question. Have you tried with the lamar image (here)? This supposed to install all the dependencies. I was wondering if having the new pycolmap version breaks the pipeline, in that case I could apply some of your changes in the Dockerfile to return to the old one. |
|
Thanks a lot for the contribution @ovysotska ! The docker image is not fully functional either as some calls to pycolmap in this pipeline are not compatible with 0.6.0. I think it's better if users can also install from scratch without needing docker so having a dedicated script for local setup would be a good thing. |
|
@pablospe thanks for the comment. I checked the About the docker image, yes I tried building it. The building part seemed to work :) at least there were no obvious errors. Thanks! |
Ok, I see. Anyways I would like to split this script in several scripts that could be reusable also in the Dockerfile. I will do/propose some changes to this PR, but essentially the same code. Thanks for the contribution! |
|
@ovysotska |
|
I will try but now I have a question. Do I need to create my own virtual environment, for example through |
Yes, if you want to install the python packages in a new environment. Otherwise, it will install them in the user's default one. |
… in the Docker image).
Notice that now all the bash scripts are in |
scripts/build_hloc.sh
Outdated
| pip wheel --no-deps -w dist-wheel . | ||
| whl_path=$(find dist-wheel/ -name "*.whl") | ||
| echo $whl_path >dist-wheel/whl_path.txt |
There was a problem hiding this comment.
Does this actually work? This would package hloc without the feature extractors/matchers, which are outside the Python package. We generally build with the -e option and can't turn this into a wheel.
There was a problem hiding this comment.
I see. Thanks for point this out!
This separation into builder and runtime Docker image stages was done to exclude development dependencies from the runtime image, thus reducing the overall image size. It appears to be built this way, but I need to verify what you're saying. I didn't actually use the Lamar image but rather the Scantools one.
Notice that the script introduced in this PR (which installs it locally) doesn't use this method. Instead, it directly runs:
python3 -m pip install git+https://github.com/cvg/[email protected]
Therefore, this issue only affects the Docker image.
There was a problem hiding this comment.
This should be resolved in the last commit. Thanks!
There was a problem hiding this comment.
My comment also holds for this installation command, since pip only retains the package files in hloc/ and discards all dependencies that are in third_party/. Only features/matchers pulled via pip (so DISK, ALIKED, LightGlue, and the image retrieval features) will be available then. I will fix this at some point but it's a low priority on my end.
There was a problem hiding this comment.
ok, thanks again. I will fix it soon.
…der/runtime, so docker images will be larger)
This PR adds a script to pull and install all core dependencies needed for the LaMAR.
Warning, the C++ dependencies will be installed system wide.
This PR also: