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

Address issues #41476

Closed
wants to merge 5 commits into from
Closed

Address issues #41476

wants to merge 5 commits into from

Conversation

civanch
Copy link
Contributor

@civanch civanch commented May 1, 2023

PR description:

Address two issues:

  1. [ASAN] heap-buffer-overflow in EcalDetailedTimeRecHitProducer::produce() #40764 : bug in resize of a vector;

  2. Clean-up of the code for R-hadron interaction with matter is needed #40477 : clean-up of the old model class for R-hadrons hadronic interactions, removed old commented lines, removed unused variables, substitute internal Poisson method by Geant4 function, simplified count of the number of nucleons, substitute exit(..) by G4Exception(..).

No change in any WF is expected.

PR validation:

private

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for: the fix of G4TimeEcal will be backported to 13_0

@civanch
Copy link
Contributor Author

civanch commented May 1, 2023

type bugfix

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41476/35340

  • This PR adds an extra 40KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master.

It involves the following packages:

  • DataFormats/EcalDigi (simulation)
  • SimG4Core/CustomPhysics (simulation)

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@missirol, @rchatter, @makortel, @rovere, @argiro, @bsunanda, @thomreis, @wang0jin, @fabiocos, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@civanch
Copy link
Contributor Author

civanch commented May 1, 2023

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-89f3a7/32269/summary.html
COMMIT: 76ca69f
CMSSW: CMSSW_13_1_X_2023-05-01-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41476/32269/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 1 lines from the logs
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3460877
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3460849
  • 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

@makortel
Copy link
Contributor

makortel commented May 1, 2023

test parameters:

  • release = CMSSW_13_1_ASAN_X
  • workflow = 21034.114

@makortel
Copy link
Contributor

makortel commented May 1, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41476/35344

  • This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41476/35345

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

Pull request #41476 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

@civanch
Copy link
Contributor Author

civanch commented May 1, 2023

please test

it seems there was a real problem in fill/access of arrays. This may change results, so we need to run standard set of tests first.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

-1

Failed Tests: UnitTests RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-89f3a7/32281/summary.html
COMMIT: 15f5210
CMSSW: CMSSW_13_1_ASAN_X_2023-04-28-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41476/32281/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-89f3a7/32281/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-89f3a7/32281/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test TestTriggerResultsFormat had ERRORS

RelVals

The relvals timed out after 4 hours.

@wddgit
Copy link
Contributor

wddgit commented May 2, 2023

The unit test failure (TestTriggerResultsFormat) is probably caused by the fact that the base IB release for this PR test invocation precedes the release where this unit test was added. The unit test depends on a new data file added to the new external data area. I think it will pick it up if you rerun the tests again. ASAN with CMSSW_13_1_X_2023-05-01-2300 should have it.

@makortel
Copy link
Contributor

makortel commented May 2, 2023

@cmsbuild, please test

@civanch
Copy link
Contributor Author

civanch commented May 2, 2023

It was a mistake to try to fix two issues in one PR. Let us test run but this will be closed and two new will be opened. Relatively simple fix #41487 is already submitted. Another will be proposed for DIGI step when 13_2_X will be available.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2023

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-89f3a7/32311/summary.html
COMMIT: 15f5210
CMSSW: CMSSW_13_1_ASAN_X_2023-05-01-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41476/32311/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

The relvals timed out after 4 hours.

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