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

[RFC] Proposal to merge the Patatrack development into CMSSW 10.5.x [squashed] #25647

Closed

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Jan 14, 2019

This should be equivalent to #25353 after fixing conflicts and squashing on top of CMSSW_10_4_0 .

Currently, not all the developments we would like to integrate have been merged in the development branch: see cms-patatrack#200 for the status of the Patatrack branch with respect to readiness for merging upstream.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@fwyzard
Copy link
Contributor Author

fwyzard commented Jan 14, 2019

enable GPU

@fwyzard
Copy link
Contributor Author

fwyzard commented Jan 14, 2019

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/7978

  • This PR adds an extra 1104KB to repository

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/7978/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/7978/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

try {
auto error = cudaGetErrorName(status);
auto message = cudaGetErrorString(status);
throw cms::Exception("CUDAError") << "Callback of CUDA stream " << streamId << " in device " << deviceId << " error " << error << ": " << message;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be replaced with std::make_exception_ptr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were using that before, but I changed to a throw in order to be able to catch throw it in GDB

Copy link
Contributor

Choose a reason for hiding this comment

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

You could change catch(..) to catch(cms::Exception const&) instead, since you know the type.

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, I see what you mean - sure, that makes sense.

@Dr15Jones
Copy link
Contributor

@Dr15Jones @makortel what should we do regarding the coding rules violations reported here, regarding the use of catch(...) in HeterogeneousCore/Producer/src/HeterogeneousEDProducer.cc and HeterogeneousCore/CUDACore/src/GPUCuda.cc ?

I think we could suppress it using https://clang.llvm.org/extra/clang-tidy/#id3
@gartung will the clang-tidy suppression also work with our static analyzer changes?

@fwyzard
Copy link
Contributor Author

fwyzard commented Jan 22, 2019

For the moment I have added a whitelist to out python checks, same as for the FWCore packages.

@fwyzard
Copy link
Contributor Author

fwyzard commented Jan 22, 2019

by the way @kpedro88, do you have any idea why I do not get the same error when testing locally, on top of the latest IB ?

@gartung
Copy link
Member

gartung commented Jan 22, 2019

@gartung will the clang-tidy suppression also work with our static analyzer

@Dr15Jones not that I know of.

@kpedro88
Copy link
Contributor

@fwyzard I am not sure, the PR that added .7 workflows has been in IBs since CMSSW_10_5_X_2019-01-16-2300.

Take into account the proper number of modules: increment by one
pathsAndConsumes.allModules().size(), because it does not include the
"source" module.

Add asserts to check that all ranges are properly closed.

Optionally, delay starting the profiler until after the first event on
each stream has completed. This requires running 'nvprof' with the
'--profile-from-start off' option.
Squash git@github.com:cms-patatrack/cmssw.git/CMSSW_10_4_X_Patatrack on
top of CMSSW_10_5_X_2019-01-23-2300 .
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/8173

  • This PR adds an extra 1160KB to repository

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/8173/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25647/8173/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

@fwyzard
Copy link
Contributor Author

fwyzard commented Jan 24, 2019

Will try again...

@fwyzard fwyzard closed this Jan 24, 2019
@fwyzard fwyzard deleted the Patatrack_developments_104x branch January 24, 2019 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment