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

Remove service named EnableFloatingPointExceptions #19065

Merged

Conversation

wddgit
Copy link
Contributor

@wddgit wddgit commented Jun 1, 2017

This was discussed and decided at the Core meeting
on 30 May 2017. We do not want to use resources maintaining
this service for several reasons. First of all, as far as we
know no one uses or has used this service and it has been
around for more than 10 years. Its unit test is broken. It
does not work properly in the new multithreaded environment
and would be difficult to fix, the precision control
parts of it are obsolete with modern processors, and it
does not work properly with some architectures.

This was discussed and decided at the Core meeting
on 30 May 2017. We do not want to use resources maintaining
this service for several reasons. First of all, as far as we
know no one uses or has used this service and it has been
around for more than 10 years. Its unit test is broken. It
does not work properly in the new multithreaded environment
and would be difficult to fix, the precision control
parts of it are obsolete with modern processors, and it
does not work properly with some architectures.
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

DataFormats/MuonDetId
FWCore/Framework
FWCore/Services
Geometry/CMSCommonData
Geometry/TrackerCommonData
L1Trigger/CSCTriggerPrimitives
SLHCUpgradeSimulations/Geometry
SimG4Core/PrintGeomInfo
Validation/Performance

@smuzaffar, @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @rekovic, @kpedro88, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @Martin-Grunewald, @kreczko, @battibass, @makortel, @abbiendi, @jhgoh, @ghugo83, @valuev, @calderona, @HuguesBrun, @ptcox, @ebrondol, @VinInn, @trocino, @venturia this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@wddgit
Copy link
Contributor Author

wddgit commented Jun 1, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20287/console Started: 2017/06/01 16:58

@slava77
Copy link
Contributor

slava77 commented Jun 1, 2017 via email

@@ -46,9 +46,6 @@
firstRun = cms.untracked.uint32(1)
)

Copy link
Contributor

Choose a reason for hiding this comment

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

@wddgit - could you, please, remove the whole file? It's been replaced by SimG4Core/PrintGeomInfo

@ianna
Copy link
Contributor

ianna commented Jun 1, 2017

+1

Note, none of these Geometry test configurations is tested in a release integration. It is fine to merge the PR as is I can clean up the obsolete configuration later.

@wddgit
Copy link
Contributor Author

wddgit commented Jun 1, 2017

At the meeting, I was wondering if anyone would say they had used this when the PR came out. I did remove every reference to the service in repository hoping any actual user would notice. As far as I know there is no alternative to the service other than manually coding this functionality yourself for whatever test you want to do. If people think maintaining this service is worth the effort, then it probably would not be very hard to fix the service so enabling floating point exceptions works when the job is configured to have only one thread and only on LINUX. The precision control part is truly obsolete on modern processors and should be deleted. To make it work properly in multithreaded jobs would take some thought. I do not know if it is just difficult or impossible ... Chris may want to comment on this when he returns from vacation on June 12th.

I have not spent time investigating why it fails on AARCH64. It does use glibc functions and has special code enabled for the case #ifdef APPLE so that those functions are defined there ...

@slava77
Copy link
Contributor

slava77 commented Jun 1, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2017

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

Comparison Summary:

  • You potentially added 14 lines to the logs
  • Reco comparison results: 1684 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1789385
  • DQMHistoTests: Total failures: 54953
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1734259
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@civanch
Copy link
Contributor

civanch commented Jun 6, 2017

+1

@davidlange6 davidlange6 merged commit d5432f4 into cms-sw:master Jun 6, 2017
@wddgit wddgit deleted the removeEnableFloatingPointExceptionsService branch September 8, 2017 21:52
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

6 participants