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 the cuda-compatible-runtime test as a new external #7279

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Sep 8, 2021

No description provided.

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2021

A new Pull Request was created by @fwyzard (Andrea Bocci) for branch IB/CMSSW_12_1_X/master.

@smuzaffar, @mrodozov, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.
cms-bot commands are listed here

cp %_builddir/build/cuda-compatible-runtime %{i}/test/cuda-compatible-runtime
else
ln -s /usr/bin/false %{i}/test/cuda-compatible-runtime
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

@fwyzard , I do not understand this logic here. This is run on the build machine, so if it fails to build then what does that mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be a system for which we don't have a CUDA version working (e.g. GCC 12 when it comes out, and before CUDA is updated for it).
If that happens, I thought it safer not to make the build fail, and just replace the script used for the check with something that will always fail (e.g. /usr/bin/false).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good news: the link to /usr/bin/false' works as intended.
Bad news: building the cuda-compatible-runtime binary failed on what should be a supported architecture :-(

Is there a way to access the build logs for this PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah... the sources on gitlab are redirecting to the login page :-(
How do I make cmsbuild log in ?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah this is coming from a private repo. Can we move the test.c to come public repo? Though the cmsbuild user has valid krb5 token but as you are downloading a single file using https protocol and that requires username/password. I do not know if there is a gitlab url which can use krb5 token to download a single file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I found a way to make it work, now I'm working through a couple other issues...

@fwyzard fwyzard force-pushed the IB/CMSSW_12_1_X/master_add_cuda-compatible-runtime_external branch from caa1a03 to 293dd8e Compare September 8, 2021 10:05
@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2021

Pull request #7279 was updated.

%define branch master
%define commit 18d5a51bfb32fbb7b765dd2eecd14e193cce8126

Source: git+https://:@gitlab.cern.ch:8443/cms-patatrack/%{n}.git?obj=%{branch}/%{commit}&export=%{n}&filter=test.c&output=/%{n}-%{realversion}.tgz
Copy link
Contributor

Choose a reason for hiding this comment

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

this still means that only users with read access to this repo can build this external ... right ? This will stop other users to build externals

Copy link
Contributor

Choose a reason for hiding this comment

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

please change the filter=test.c to filter=./test.c . cmsBuild uses https://github.com/cms-sw/pkgtools/blob/V00-32-XX/cmsBuild#L531 to filter the files which means they will have ./ infront

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021 via email

@fwyzard fwyzard force-pushed the IB/CMSSW_12_1_X/master_add_cuda-compatible-runtime_external branch from 293dd8e to 40eb66c Compare September 8, 2021 11:03
@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2021

Pull request #7279 was updated.

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021

If this works, the next step would be to replace $CMSSW_BASE/config/SCRAM/hooks/runtime/00-nvidia-drivers according to cms-sw/cmssw-config#85 .

@smuzaffar
Copy link
Contributor

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 8, 2021

Done.

@fwyzard fwyzard force-pushed the IB/CMSSW_12_1_X/master_add_cuda-compatible-runtime_external branch from 40eb66c to 9a9f0c3 Compare September 9, 2021 05:25
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2021

Pull request #7279 was updated.

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 9, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9a8f2f/18437/summary.html
COMMIT: 9a9f0c3
CMSSW: CMSSW_12_1_X_2021-09-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/7279/18437/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9a8f2f/18437/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9a8f2f/18437/git-merge-result

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 3001001
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3000973
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor

+externals

@smuzaffar smuzaffar merged commit 56ca74d into cms-sw:IB/CMSSW_12_1_X/master Sep 9, 2021
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2021

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_12_1_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants