Skip to content

[15_0_X] fix:Duplicate DeepCSV path removal, onnx model update #48853

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.

Sign up for GitHub

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

Jump to bottom
Merged
cmsbuild merged 1 commit into from
Sep 9, 2025
Merged

[15_0_X] fix:Duplicate DeepCSV path removal, onnx model update #48853

cmsbuild merged 1 commit into from
Sep 9, 2025

Conversation

@uttiyasarkar
Copy link
Contributor

@uttiyasarkar uttiyasarkar commented Sep 5, 2025

PR description:

This is a backport of PR#48256 which includes the fixed DeepJet model in Phase-2 menu with corrected softmax layer and duplicate DeepCSV paths being removed

@cmsbuild cmsbuild added this to the CMSSW_15_0_X milestone Sep 5, 2025
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2025 •
edited
Loading

A new Pull Request was created by @uttiyasarkar for CMSSW_15_0_X.

It involves the following packages:

  • HLTrigger/Configuration (hlt)

@Martin-Grunewald, @cmsbuild, @mmusich can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @SohamBhattacharya, @VourMa, @missirol, @mmusich, @rovere this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2025 •
edited
Loading

cms-bot internal usage

@mmusich
Copy link
Contributor

mmusich commented Sep 5, 2025

@uttiyasarkar this needs to be tested with a backport of cms-sw/cmsdist#9918 to the IB/CMSSW_15_0_X/master branch of cmsdist. Please provide one.

@mmusich
Copy link
Contributor

mmusich commented Sep 5, 2025

backport of #48256

@uttiyasarkar uttiyasarkar mentioned this pull request Sep 5, 2025
Merged
@mmusich
Copy link
Contributor

mmusich commented Sep 6, 2025

test parameters:

@mmusich
Copy link
Contributor

mmusich commented Sep 6, 2025

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2025

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-34f7f9/47994/summary.html
COMMIT: e2c313a
CMSSW: CMSSW_15_0_X_2025-09-06-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48853/47994/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 17 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 38 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4020850
  • DQMHistoTests: Total failures: 33
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4020797
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 218 log files, 189 edm output root files, 50 DQM output files
  • TriggerResults: found differences in 4 / 48 workflows

mmusich
mmusich reviewed Sep 8, 2025
View reviewed changes
HLTrigger/Configuration/python/HLT_75e33_cff.py
fragment.HLT_PFPuppiMETTypeOne140_PFPuppiMHT140,

fragment.L1T_PFHT400PT30_QuadPFPuppiJet_70_55_40_40_2p4,
fragment.L1T_DoublePFPuppiJets112_2p4_DEta1p6,
Copy link
Contributor

@mmusich mmusich Sep 8, 2025

Choose a reason for hiding this comment

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

the deletion of these two lines are not included in the master PR version #48256.
Please clarify the reason of the deletion.

Copy link
Contributor Author

@uttiyasarkar uttiyasarkar Sep 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am wondering why these two lines are deleted (probably accidentally removed while resolving the conflict, sorry about this). I can add them back in a commit

Copy link
Contributor

@mmusich mmusich Sep 8, 2025

Choose a reason for hiding this comment

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

can you please put them back where they were (L362-363) and not later (L367-368)?
Also please squash the commits.

Copy link
Contributor Author

@uttiyasarkar uttiyasarkar Sep 8, 2025

Choose a reason for hiding this comment

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

Everything should ideally be resolved

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

Pull request #48853 was updated. @Martin-Grunewald, @cmsbuild, @mmusich can you please check and sign again.

1 similar comment
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

Pull request #48853 was updated. @Martin-Grunewald, @cmsbuild, @mmusich can you please check and sign again.

@uttiyasarkar
fix:removal from the scheduler
3a80e7f 
fix:Duplicate DeepCSV path removal, onnx model update

bug:readded deleted L1 fragments

Fix: onnx update and deepcsv duplicate path removal
@uttiyasarkar uttiyasarkar force-pushed the backport-PR48256-to-CMSSW_15_0_X branch from 62f6ba9 to 3a80e7f Compare September 8, 2025 12:40
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

Pull request #48853 was updated. @Martin-Grunewald, @cmsbuild, @mmusich can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Sep 8, 2025

@cmsbuild, please test

@mmusich
Copy link
Contributor

mmusich commented Sep 8, 2025

type bug-fix

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-34f7f9/48011/summary.html
COMMIT: 3a80e7f
CMSSW: CMSSW_15_0_X_2025-09-08-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/48853/48011/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 15 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 40 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4020850
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4020827
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 218 log files, 189 edm output root files, 50 DQM output files
  • TriggerResults: found differences in 4 / 48 workflows

@mmusich
Copy link
Contributor

mmusich commented Sep 8, 2025

+hlt

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

This pull request is fully signed and it will be integrated in one of the next CMSSW_15_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_15_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @ftenchini, @mandrenguyen, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: cms-sw/cmsdist#10060

@mandrenguyen mandrenguyen changed the title Backport PR#48256 to CMSSW_15_0_X [15_0_X] fix:Duplicate DeepCSV path removal, onnx model update Sep 8, 2025
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2025

REMINDER @mandrenguyen, @ftenchini, @sextonkennedy: This PR was tested with cms-sw/cmsdist#10060, please check if they should be merged together

@mandrenguyen
Copy link
Contributor

mandrenguyen commented Sep 9, 2025

+1

@cmsbuild cmsbuild merged commit cd7ca8a into cms-sw:CMSSW_15_0_X Sep 9, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mmusich mmusich mmusich left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport-ok bug-fix fully-signed hlt-approved orp-approved requires-external tests-approved

Projects

None yet

Milestone

CMSSW_15_0_X

Development

Successfully merging this pull request may close these issues.

4 participants

@uttiyasarkar @cmsbuild @mmusich @mandrenguyen