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

Require benchmark 1.5.4, update nightly containers to Ubuntu 22.04 #799

Merged
merged 5 commits into from
Dec 28, 2022

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented Dec 26, 2022

  • Require benchmark 1.5.4
  • Update our BVH benchmark to use new rate feature
  • Update nightly containers to Ubuntu 22.04

Still no way to require specific version of the benchmark in CMake, though. The main reason for the PR is to not propagate this rate issue to other benchmarks, such as union-find.

@aprokop aprokop added build Build and installation testing Anything to do with tests and CI labels Dec 26, 2022
@dalg24
Copy link
Contributor

dalg24 commented Dec 27, 2022

not propagate this rate issue to other benchmarks, such as union-find.

I do not follow. What do you mean?

@aprokop
Copy link
Contributor Author

aprokop commented Dec 27, 2022

I do not follow. What do you mean?

In the bvh_driver we had a comment about how we would have used a kIsIterationInvariantRate feature but it's only available in 1.5. I would have added a similar FIXME to the union-find benchmark. It's not a big deal, but also a debt.

@dalg24
Copy link
Contributor

dalg24 commented Dec 27, 2022

v1.5.0 was released in May 2019 (vs May 2018 for v1.4.1)
One thing to check is whether that would break our nightly builds because they install libbenchmark-dev using the system package manager. Both builds are Ubuntu20.04-based but we could upgrade to Ubuntu22.04 images as needed.

Another thing to consider is whether to adopt a more recent version that has the package version set. For the record, support was added in google/benchmark#1047

@aprokop
Copy link
Contributor Author

aprokop commented Dec 27, 2022

One thing to check is whether that would break our nightly builds because they install libbenchmark-dev using the system package manager. Both builds are Ubuntu20.04-based but we could upgrade to Ubuntu22.04 images as needed.

Yes, it will break the builds. Ubuntu20.04 comes with benchmark 1.5.0.

Another thing to consider is whether to adopt a more recent version that has the package version set. For the record, support was added in google/benchmark#1047

I think it's a good idea. The first version of the benchmark that includes that PR is 1.5.4.

@aprokop aprokop changed the title Require benchmark 1.5 with a new rate feature Require benchmark 1.5.4 Dec 27, 2022
@aprokop aprokop changed the title Require benchmark 1.5.4 Require benchmark 1.5.4, update nightly containers to Ubuntu 22.04 Dec 27, 2022
@aprokop aprokop force-pushed the require_benchmark_1.5 branch 2 times, most recently from 0125500 to ddfe298 Compare December 27, 2022 17:31
agent {
docker {
image 'nvidia/cuda:11.4.2-devel-ubuntu20.04'
image 'nvidia/cuda:11.7.1-devel-ubuntu22.04'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 11.7.1?
For the record, looking at docker hub https://hub.docker.com/r/nvidia/cuda/tags?page=1&name=ubuntu22, we can chose from [11.7.0, 11.7.1, 11.8.0, 12.0.0]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, out of those versions, I wanted the earlierst, but thought that maybe 11.7.1 fixed some bugs in 11.7.0.

agent {
docker {
image 'rocm/dev-ubuntu-20.04:5.2-complete'
image 'rocm/dev-ubuntu-22.04:5.4-complete'
Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. For the record we have to chose from [5.3, 5.4]

.gitlab-ci.yml Show resolved Hide resolved
Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Make sure you update the wiki

@aprokop
Copy link
Contributor Author

aprokop commented Dec 28, 2022

Was accidentally closed by 803.

Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Maybe copy/paste the nightly builds into the continuous to check that it actually works

@aprokop
Copy link
Contributor Author

aprokop commented Dec 28, 2022

Apparently apt is broken in the new CUDA images used for nightly. apt-get update fails with

W: GPG error: http://archive.ubuntu.com/ubuntu jammy-updates InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 871920D1991BC93C

@aprokop
Copy link
Contributor Author

aprokop commented Dec 28, 2022

Maybe copy/paste the nightly builds into the continuous to check that it actually works

Yeah, was trying to update the wiki to see what versions it carries, and ran into issues.

@Rombur
Copy link
Collaborator

Rombur commented Dec 28, 2022

Yes, I have seen this before https://stackoverflow.com/questions/71941032/why-i-cannot-run-apt-update-inside-a-fresh-ubuntu22-04 We need to update docker to use a ubuntu 22.04 image

@aprokop
Copy link
Contributor Author

aprokop commented Dec 28, 2022

Yes, I have seen this before stackoverflow.com/questions/71941032/why-i-cannot-run-apt-update-inside-a-fresh-ubuntu22-04 We need to update docker to use a ubuntu 22.04 image

The error I'm getting is different, though, and related to the keys. On my workstation, I'm running docker 18.09.6 🤷‍♂️

@aprokop
Copy link
Contributor Author

aprokop commented Dec 28, 2022

Apparently apt is broken in the new CUDA images used for nightly. apt-get update fails with

Nevermind. Apparently, it's only broken on my machine. The nightly builds pass just fine: https://cloud.cees.ornl.gov/jenkins-ci/blue/organizations/jenkins/ArborX/detail/PR-799/5/pipeline.

@dalg24 No issues.

@dalg24 dalg24 merged commit 9799ca8 into arborx:master Dec 28, 2022
@aprokop aprokop deleted the require_benchmark_1.5 branch December 28, 2022 18:16
@aprokop aprokop mentioned this pull request Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build and installation testing Anything to do with tests and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants