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

Add support for CUDA in CI #4101

Merged
merged 1 commit into from
May 30, 2020

Conversation

shrijitsingh99
Copy link
Contributor

Updated Dockerfile for CUDA 10.2 support
Updated Eigen version to resolve CUDA conflict
Updated GCC version to resolved build errors in GPU module

@kunaltyagi kunaltyagi added changelog: new feature Meta-information for changelog generation module: ci needs: code review Specify why not closed/merged yet labels May 15, 2020
@kunaltyagi
Copy link
Member

  • The CUDA version should also be updated in the CI which generates (and overrides) the version in the file
  • Eigen3 is being installed regardless
  • If default gcc-x, x> 7, no need to install gcc-7

@shrijitsingh99
Copy link
Contributor Author

Should I also bump the librealsense version while I am at it?

@SergioRAgostinho
Copy link
Member

Should I also bump the librealsense version while I am at it?

Ideally, only for the 20.04 build I would say. 16.04 addresses the lowest possible versions and 20.04 the highest. Thoughts?

@shrijitsingh99
Copy link
Contributor Author

shrijitsingh99 commented May 15, 2020

Ideally, only for the 20.04 build I would say. 16.04 addresses the lowest possible versions and 20.04 the highest. Thoughts?

Sounds good, Tracking these versions as you mentioned on #4099, would allow us to identify and bump versions accordingly, maybe in every release, not sure.

@shrijitsingh99
Copy link
Contributor Author

Will bump real sense version in a seperate PR.

.dev/docker/env/Dockerfile Outdated Show resolved Hide resolved
.dev/docker/env/Dockerfile Outdated Show resolved Hide resolved
@kunaltyagi kunaltyagi added needs: more work Specify why not closed/merged yet and removed needs: code review Specify why not closed/merged yet labels May 18, 2020
.ci/azure-pipelines/build-ubuntu.yaml Outdated Show resolved Hide resolved
.dev/docker/env/Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but not to merge

This actually paints the CI red, so adding more PR to resolve the issues would be preferred (rather than changes in this one)

@kunaltyagi
Copy link
Member

Oh, and the docker images should be updated separate to the CI. That actually allows the pipeline to update them.

@kunaltyagi kunaltyagi added needs: pr merge Specify why not closed/merged yet and removed needs: more work Specify why not closed/merged yet labels May 20, 2020
@shrijitsingh99
Copy link
Contributor Author

Okay, so let me split this into 3 PR's then:

  1. Add clang CI to Ubuntu 18
  2. Update Docker image
  3. Integrate GPU builds with a new image (This PR)
    We can merge them in the order specified. Does that sound good?

@kunaltyagi
Copy link
Member

We can merge them in the order specified

Number 2 has a very high chance of being 1st to be accepted

@shrijitsingh99
Copy link
Contributor Author

This should be good to do now. Have rebased with master.

@kunaltyagi kunaltyagi added needs: code review Specify why not closed/merged yet and removed needs: pr merge Specify why not closed/merged yet labels May 30, 2020
@kunaltyagi kunaltyagi merged commit 07a19d0 into PointCloudLibrary:master May 30, 2020
@shrijitsingh99 shrijitsingh99 deleted the cuda-ci branch June 11, 2020 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: new feature Meta-information for changelog generation module: ci needs: code review Specify why not closed/merged yet priority: gsoc Reason for prioritization
Projects
None yet
3 participants