-
Notifications
You must be signed in to change notification settings - Fork 23
Add dependencies and Pixi configuration simpler project setup #4
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
base: main
Are you sure you want to change the base?
Conversation
- Updated `pyproject.toml` to include new dependencies: timm, cupy-cuda12x, cholespy, html4vision, geffnet, and mmcv-lite. - Configured Pixi workspace with conda-forge channel and specified platforms. - Added PyPI options and dependencies for mpsfm, torch, torchvision, xformers, simplecv, lightglue, and colmap-rerun. - Defined Pixi tasks for cloning and building colmap, as well as installing pycolmap. - Specified additional dependencies for the project, including rerun-sdk, omegaconf, h5py, and others.
Issue:
|
Apologies, I accidentally committed that when I shouldn't have. I was doing some testing with that repo. It should not be a part of functionality. I've removed it, give it a try and see if it works for you now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of using Pixi, but a few changes are needed. The dependencies in project.toml
are quite restrictive and include packages that don’t appear essential. If we want to include this in the main branch, we’d need to simplify the file and omit pixi.lock
entirely (see my line-specific comments).
Would you be open to making those changes? If not, we could consider merging this into a separate branch or using it as a reference for alternative installation.
If you’re short on time, I might check this out myself.
depends-on = ["_build-colmap"] | ||
|
||
[tool.pixi.dependencies] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It seems like not all of these dependencies belong in this repo. For example,
rerun-sdk
doesn’t seem like a corempsfm
dependency. Also, I don’t recognizelibgomp
— is it actually required? - The version pins are very tight. For example,
python == 3.11
could unnecessarily restrict environments. Can Pixi handle more relaxed version ranges?
Overall, this would need to be significantly simplified before merging into the main branch.
@@ -3,6 +3,14 @@ name = "mpsfm" | |||
version = "0.1.0" | |||
authors = [{ name = "Zador Pataki", email = "zadorpataki14@gmail.com" }] | |||
requires-python = ">=3.9" | |||
dependencies = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear why we have both dependencies = [...]
and [tool.pixi.dependencies]
. I assume everything currently in requirements.txt
should be moved into the core here instead of under Pixi. The rest, if needed, can stay in [tool.pixi.dependencies]
.
index-strategy = "unsafe-best-match" | ||
|
||
[tool.pixi.pypi-dependencies] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CUDA versions are hardcoded to a fixed 12.8 version — this is still considered modern, and likely underutilized. Is it possible to relax this requirement? Since we are considering offering a flexible installation mechanism (Pixi), it would make sense to avoid such strict constraints. One idea could be to make these dependencies optional, e.g.:
[project.optional-dependencies]
torch = ["torch>=2.1"]
This way, users can install torch manually based on the CUDA version supported by their system’s driver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? I’m not keen on committing such a large file to the repo.
Sadly, I am a little short on time. I can maybe get back to this next week or the week after that if you're not in a rush |
To make things easier for myself, I added the option to use Pixi as an install method. Everything should still work with the original installation instructions. This makes it so that in one command, one builds + installs the custom pycolmap branch and includes most (conda doesn't currently include glew) required deps.
Pixi is a modern package manager for the conda ecosystem. It's much faster than conda/mamba and also supports lockfiles natively. Part of what makes it so useful in this case is the addition of tasks that build/install colmap and pycolmap.
Let me know if this looks interesting for you to merge if not, no problem. Just makes it easier for me to consistently get this working without having to install a bunch of things onto my apt package manager (rather have a separate conda environment for it)
TLDR easy use:
curl -fsSL https://pixi.sh/install.sh | sh
pixi run reconstruct-example
This will git clone modified colmap, build it, install pycolmap, and run python reconstruct.py!
Also means if you jump into the shell
pixi shell
and run the notebook example, everything will work there too