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

fastjet 3.4.1 and fastjet-contrib 1.051 #8485

Merged
merged 1 commit into from
May 3, 2023

Conversation

smuzaffar
Copy link
Contributor

Update fastjet 3.4.1 with bug fixes

- Resolved MAJOR BUG that arose with full thread-safety enabled (reported by Ludo Scyboz),
where PseudoJet::reset_momentum and PseudoJet::operator= failed to update internally cached
values of phi() and rap(). In some instances related issues could also lead to race conditions.
- eliminated NaN from square-root of negative mean areas in background estimation (now returns zero) 

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2023

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

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

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a7434/32304/summary.html
COMMIT: ee0f616
CMSSW: CMSSW_13_1_X_2023-05-02-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/8485/32304/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-1a7434/32304/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a7434/32304/git-merge-result

Comparison Summary

Summary:

  • You potentially removed 409 lines from the logs
  • Reco comparison results: 26110 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3460877
  • DQMHistoTests: Total failures: 63790
  • DQMHistoTests: Total nulls: 6
  • DQMHistoTests: Total successes: 3397059
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.023 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 4.53 ): 0.023 KiB JetMET/SUSYDQM
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 2 / 46 workflows

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a7434/32325/summary.html
COMMIT: ee0f616
CMSSW: CMSSW_13_1_X_2023-05-02-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/8485/32325/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-1a7434/32325/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a7434/32325/git-merge-result

Comparison Summary

Summary:

  • You potentially added 4 lines to the logs
  • Reco comparison results: 12 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3460877
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3460843
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor Author

@perrotta @rappoccio is it too late to get this in for 13.1.X ?
@makortel , this version of fastjet has a bug fix for thread safety but I am not sure if we are affected by this

@perrotta
Copy link
Contributor

perrotta commented May 3, 2023

@smuzaffar I intended to build pre4 ~now
Since I did not start it yet, it is not (yet) too late
If the inclusion is safe, I think we can merge it and start building pre4 right after: just decide quickly, feel free to consider "+orp" here

@smuzaffar
Copy link
Contributor Author

+externals
PR tests look good, DQM/Reco comparison show differences in MessageLogger, so let get this in then.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2023

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_13_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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@smuzaffar
Copy link
Contributor Author

Thanks @perrotta , I have merged it now. If it helps then I can start an IB too

@perrotta
Copy link
Contributor

perrotta commented May 3, 2023

If it helps then I can start an IB too

Do you think it is needed?
I would start the build o pre4 now anyhow, but we can still have a look at the results of the IB tests before the uploads

@smuzaffar
Copy link
Contributor Author

11h00 IB does not contain this change, so you have to wait for 23h00 IB results. If this is fine then no need to build an extra IB

@makortel
Copy link
Contributor

makortel commented May 3, 2023

this version of fastjet has a bug fix for thread safety but I am not sure if we are affected by this

Good question. I don't recall seeing fastjet-specific thread safety problems in a long time, but I don't expect our testing to be able catch all possible problems :) Since the comparisons were clean, I'd be tempted to backport these fixes to 13_0_X (but maybe after 13_1_0_pre4 has been validated?

@smuzaffar smuzaffar deleted the fastjet-3.4.1 branch May 8, 2023 07:29
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.

4 participants