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

Update to Colmap 3.9.1 and added Dockerfile. #26

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

pablospe
Copy link
Contributor

This PR updates the changes made to Colmap 3.9 and adds a Dockerfile. Usage:

export COLMAP_VERSION=3.9.1
export CUDA_ARCHITECTURES=70
docker build -t pyceres \
    --build-arg COLMAP_VERSION=${COLMAP_VERSION}  \
    --build-arg CUDA_ARCHITECTURES=${CUDA_ARCHITECTURES}  \
    -f Dockerfile .

A possible improvement for a future pull request is to create a runtime docker stage that only installs the essential dependencies and uses the compiled ceres-solver and colmap from the builder stage (this will reduce the docker image size considerably).

@pablospe
Copy link
Contributor Author

@Phil26AT @sarlinpe I was wondering if you could take a look to this PR.

@sarlinpe
Copy link
Member

LGTM. I don't use Docker so I don't really understand all these details.

@sarlinpe sarlinpe merged commit 533405d into cvg:main Jan 24, 2024
@sarlinpe
Copy link
Member

@pablospe Since COLMAP 3.9.1 the quaternions follow the Eigen format (xyzw instead of wxyz) but this wasn't updated here - so the user need to manually handle any conversion.

@pablospe
Copy link
Contributor Author

Yes, you are right, this is not handle it here. Should we revert these changes?

@sarlinpe
Copy link
Member

I'd rather revert for now and do a proper upgrade in #27 - can you submit one and keep the Docker stuff if you need it?

@pablospe
Copy link
Contributor Author

Yes, it makes sense to revert. I'll create another PR with only the Dockerfile. You could also take the changes from this PR, like the rename of WorldToImage for ImgFromCam: see in bundle.cc.

@pablospe
Copy link
Contributor Author

One question, it only uses only the camera models from Colmap, right? I was wondering if it would make sense to simply take a copy the models.h to avoid Colmap dependency in this repo? It is like a circular dependency, for compiling Colmap you need ceres, and for pyceres you need Colmap :)

@sarlinpe
Copy link
Member

My plan is to move the bundle & PGO cost functions to pycolmap, such that pyceres handles only the core features without any dependency on COLMAP. But this requires some updates upstream to add covariances.

@pablospe
Copy link
Contributor Author

Related PR (for the Dockerfile part): #31

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.

2 participants