Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pablovela5620
Copy link

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:

  1. Install pixi curl -fsSL https://pixi.sh/install.sh | sh
  2. run command 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

- 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.
@shuta-ochiai
Copy link

Issue: pixi install Failure - Potential colmap-rerun Path Issue

Hi @pablovela5620

Thank you for your very useful PR!
I'm currently trying to build, and I'm running into an issue with pixi install.

The error message I'm seeing is:

Error: × failed to solve the pypi requirements of 'default' 'linux-64' ├─▶ error while canonicalization /app/mpsfm/../colmap-rerun ╰─▶ No such file or directory (os error 2)

It seems like there might be a problem with resolving the path to colmap-rerun. I noticed that pyproject.toml defines colmap-rerun as being located in the parent directory of mpsfm.

Is colmap-rerun a required dependency for the core functionality? If so, it might be necessary to ensure correctly setting up the directory structure so that pixi install can find it. If it's not strictly required, perhaps temporarily removing it from pyproject.toml could help unblock the build.

Any insights you could provide on this would be greatly appreciated!

@pablovela5620
Copy link
Author

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

Copy link
Collaborator

@Zador-Pataki Zador-Pataki left a 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]
Copy link
Collaborator

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 core mpsfm dependency. Also, I don’t recognize libgomp — 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 = [
Copy link
Collaborator

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]
Copy link
Collaborator

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.

Copy link
Collaborator

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.

@pablovela5620
Copy link
Author

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

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.

3 participants