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

use openblas instead of gslcblas #5063

Merged
merged 1 commit into from Jul 4, 2019
Merged

Conversation

smuzaffar
Copy link
Contributor

  • Build OpenBlas with DYNAMIC_ARCH=1
  • Drop gslcblas library from gsl toolfile and add openblas dependency to make sure that any cmssw package using gsl links openblas instead of gslcblas
  • set GSL_CBLAS_LIB so that gsl-config properly return -lopenblas instead of -lgslcblas
  • Fixed geneva, thepeg and herwigpp specs to link against openblas

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 28, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1253/console Started: 2019/06/28 17:40

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for branch IB/CMSSW_11_0_X/gcc700.

@cmsbuild, @smuzaffar, @gudrutis, @mrodozov can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@smuzaffar smuzaffar changed the title use openblls instead of gslcblas use openblas instead of gslcblas Jun 28, 2019
@cmsbuild
Copy link
Contributor

-1

Tested at: a849fb5

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fda669/1253/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test DetectorDescriptionRegressionTestDOMCount had ERRORS

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fda669/1253/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3255670
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3255334
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@slava77
Copy link
Contributor

slava77 commented Jun 29, 2019

There is one small difference in a jet b-tag discriminant variable


WF # | Differences
-- | --
all_OldVSNew_RunJetHT2016EreMINIAODwf136p7611 | 1

It's hard to say which one (I only plot by the vector index); it may even be one of the mxnet-based cases.
@hqucms we may need some further investigation.

@smuzaffar please clarify if this change (alone, or at least without much else related) can be made available in the DEVEL branch IBs.
Thank you.

@smuzaffar
Copy link
Contributor Author

@slava77 , DEVEL IBs are broken due to tensorflow update. I can include it in gcc8 or root 618 IBs

@hqucms
Copy link
Contributor

hqucms commented Jul 2, 2019

There is one small difference in a jet b-tag discriminant variable


WF # | Differences
-- | --
all_OldVSNew_RunJetHT2016EreMINIAODwf136p7611 | 1

It's hard to say which one (I only plot by the vector index); it may even be one of the mxnet-based cases.
@hqucms we may need some further investigation.

@slava77 Is this difference in AK4 jets or AK8 jets? Only AK8 jets have MXNet-based taggers.

@slava77
Copy link
Contributor

slava77 commented Jul 2, 2019

@slava77 Is this difference in AK4 jets or AK8 jets? Only AK8 jets have MXNet-based taggers.

in AK8

@hqucms
Copy link
Contributor

hqucms commented Jul 2, 2019

@slava77 Is this difference in AK4 jets or AK8 jets? Only AK8 jets have MXNet-based taggers.

in AK8

OK I will take a look.

@hqucms
Copy link
Contributor

hqucms commented Jul 4, 2019

@slava77 I confirm that there are some small numerical changes on the MXNet-based tagger outputs after changing the BLAS library. The differences are small enough so they should not have any impacts on the physics, but will unfortunately affect the reproducibility of existing NanoAOD workflows.

@slava77
Copy link
Contributor

slava77 commented Jul 4, 2019

@hqucms
thank you for checking.

This sounds "good enough" for 11_X then.
Perhaps if all is good after a relval cycle we could consider a backport.

@smuzaffar
Copy link
Contributor Author

+externals
@fabiocos , would you like to get it in 11.0.0.pre3?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_11_0_X/gcc700 IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Jul 4, 2019

@smuzaffar @slava77 @hqucms how urgent is this/ i have already started the build, of course we can integrate, launch an IB and restart in parallel, if this is felt as urgent

@slava77
Copy link
Contributor

slava77 commented Jul 4, 2019

@smuzaffar @slava77 @hqucms how urgent is this/ i have already started the build, of course we can integrate, launch an IB and restart in parallel, if this is felt as urgent

In the context of potential backports and the next pre potentially falling into August and slow response from validators (in August), it would be nice to have it in pre3.

@fabiocos
Copy link
Contributor

fabiocos commented Jul 4, 2019

+1

@smuzaffar I would suggest to start a build with package integrated, and restart the 11_0_0_pre3 build with it

@smuzaffar
Copy link
Contributor Author

sure @fabiocos , I am triggering an IB now.

@smuzaffar smuzaffar merged commit a968bd5 into IB/CMSSW_11_0_X/gcc700 Jul 4, 2019
@fabiocos
Copy link
Contributor

fabiocos commented Jul 4, 2019

@smuzaffar thanks

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

5 participants