Skip to content

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