Hydjet++ 2.4.3 integration #36316

wouf · 2021-12-01T12:29:18Z

PR description:

Hydjet2 (HYDJET++) v2.4.3 integration as external generator.
Documentation for HYDJET++: http://lokhtin.web.cern.ch/lokhtin/hydjet++/documentation/
The minor changes in the PYQUEN and HYDJET v. 1.9.1 interfaces.

PR validation:

Hydjet2 - wf 160 with external lib for HYDJET++ cmsdist#7483
Hydjet - wf 150
Pyquen - wf 300

cmsbuild · 2021-12-01T13:02:20Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27053

This PR adds an extra 136KB to repository

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

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27053/code-checks.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27053/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly
code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27053/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27053/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-12-01T15:40:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27059

This PR adds an extra 20KB to repository

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27059/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27059/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-12-01T16:01:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36316/27061

This PR adds an extra 40KB to repository

cmsbuild · 2021-12-01T16:01:46Z

A new Pull Request was created by @wouf for master.

It involves the following packages:

Configuration/Applications (operations)
Configuration/Generator (generators)
Configuration/PyReleaseValidation (pdmv, upgrade)
GeneratorInterface/Hydjet2Interface (generators)
GeneratorInterface/HydjetInterface (generators)
GeneratorInterface/PyquenInterface (generators)

@SiewYan, @perrotta, @mkirsano, @jordan-martins, @bbilin, @wajidalikhan, @Saptaparna, @cmsbuild, @AdrianoDee, @srimanob, @agrohsje, @kskovpen, @alberto-sanchez, @qliphy, @GurpreetSinghChahal, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@mkirsano, @makortel, @cbaus, @kpedro88, @Martin-Grunewald, @missirol, @alberto-sanchez, @fabiocos, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

srimanob · 2021-12-04T23:23:59Z

test parameters:

workflows = 160
pull_request = external lib for HYDJET++ cmsdist#7483

cmsbuild · 2022-02-08T09:43:48Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2022-02-08T09:45:18Z

+1

wouf · 2022-02-08T10:05:09Z

I can suggest it right now (for memory leak we have just to free dynamicaly allocated memory). Could You please to explain how to commit such patch?

perrotta · 2022-02-08T10:22:57Z

I can suggest it right now (for memory leak we have just to free dynamicaly allocated memory). Could You please to explain how to commit such patch?

@wouf this PR is now merged. Just wait for the first IB which includes it (you can keep an eye on https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/ib/CMSSW_12_3_X for it) and implement your fixes based on that IB. Thank you!

wouf · 2022-02-08T10:23:10Z

I see, thanks!

wouf · 2022-02-08T12:10:39Z

@SiewYan is something from #36574 should be added to the patch?

SiewYan · 2022-02-08T13:21:30Z

Hello @wouf , thanks for the reminder. To put it into context, there are on-going efforts initiated from:

Migrate legacy EDModules to thread-efficient alternatives #36404
Migrate ED modules to use esConsumes #31061
to locate and migrate legacy EDModules to thread-efficient alternatives.

If the migration resume, will it break the integration effort as you mentioned in #36574 (comment)?

perrotta · 2022-02-08T16:50:26Z

@wouf this PR is already available in CMSSW_12_3_X_2022-02-08-1100
By the way, in that IB there is a crash in wf 148.0 (HydjetQ_MinBias_XeXe_5442GeV_2017) which is quite likely related to this PR, and it should be also fixed:

----- Begin Fatal Exception 08-Feb-2022 [15:](https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc11/CMSSW_12_3_X_2022-02-08-1100/pyRelValMatrixLogs/run/148.0_HydjetQ_MinBias_XeXe_5442GeV_2017+HydjetQ_MinBias_XeXe_5442GeV_2017+DIGIHI2017+RECOHI2017+HARVESTHI2017/step1_HydjetQ_MinBias_XeXe_5442GeV_2017+HydjetQ_MinBias_XeXe_5442GeV_2017+DIGIHI2017+RECOHI2017+HARVESTHI2017.log#/15)13:48 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=HydjetGeneratorFilter label='generator'
Exception Message:
ValueError type of embeddingMode is expected to be int but declared as bool
----- End Fatal Exception ----------

wouf · 2022-02-08T19:11:09Z

@perrotta I will correct, thanks! It's related this config.

@SiewYan It's already merged in CMSSW_12_3_X_2022-02-08-1100. Does the changes from Generator Interface/Hydjet2Interface/test/Hydjet2Analyzer.cc is relevant? Or maybe You can look threw Hydjet2Analyzer.cc, HydjetAnalyzer.cc and PyquenAnalyzer.cc from CMSSW_12_3_X_2022-02-08-1100? As it now, it gives warning:

>> Compiling edm plugin /afs/cern.ch/work/w/wouf/TMP/2git/fix/CMSSW_12_3_X_2022-02-08-1100/src/GeneratorInterface/PyquenInterface/test/PyquenAnalyzer.cc
In file included from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-08-1100/src/FWCore/ParameterSet/interface/ParameterSetDescriptionFiller.h:27,
                 from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-08-1100/src/FWCore/ParameterSet/interface/ParameterSetDescriptionFillerPluginFactory.h:25,
                 from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-08-1100/src/FWCore/Framework/interface/MakerMacros.h:6,
                 from /afs/cern.ch/work/w/wouf/TMP/2git/fix/CMSSW_12_3_X_2022-02-08-1100/src/GeneratorInterface/PyquenInterface/test/PyquenAnalyzer.cc:12:
/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-08-1100/src/FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h:106:65: warning: 'EDAnalyzer' is deprecated [-Wdeprecated-declarations]
  106 |     static const std::string& extendedBaseType(EDAnalyzer const*) { return kExtendedBaseForEDAnalyzer; }
      |                                                                 ^
/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-08-1100/src/FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h:107:65: warning: 'EDProducer' is deprecated [-Wdeprecated-declarations]
  107 |     static const std::string& extendedBaseType(EDProducer const*) { return kExtendedBaseForEDProducer; }
      |                                                                 ^

SiewYan · 2022-02-09T05:50:36Z

@wouf , those changes are relevant to suppress those warning messages. Is that possible to open a PR, implement it on top of this PR? I can do that if you needed help.

Please have a look at https://twiki.cern.ch/twiki/bin/view/CMSPublic/FWMultithreadedAnalysisEDAnalyzer#Legacy_EDAnalyzer.

perrotta · 2022-02-09T06:31:49Z

@wouf indeed there are still quite several other places in which embeddingMode is inserted in the configs as a bool:
https://cmssdt.cern.ch/lxr/search?v=CMSSW_12_3_X_2022-02-07-2300&_casesensitive=1&_string=embeddingMode&_filestring=.py&%21v=CMSSW_12_3_X_2022-02-08-2300

Please have a look at all them, and fix whenever needed. That could even go in a dedicated PR only meant to fix it, if it will be easier and faster to you doing as such instead of combining with all other updates and cleanings discussed here above.

(While you do so, please also take care of removing the parameter numQuarkFlavor from those configs: it is said "to be removed", and in fact it is not needed any more in the modules since a while: we can profit of this fix to clean them all from the configs.)

wouf · 2022-02-09T07:09:11Z

@perrotta I'm not sure that configs like Configuration/Generator/python/Pyquen2013Settings_cff.py still needed. It's more reasonable to delete it if it does not used any more. How to check all dependencies?

wouf · 2022-02-09T07:29:37Z

@wouf , those changes are relevant to suppress those warning messages. Is that possible to open a PR, implement it on top of this PR? I can do that if you needed help.

@SiewYan yes, could You please!

perrotta · 2022-02-09T08:38:19Z

@perrotta I'm not sure that configs like Configuration/Generator/python/Pyquen2013Settings_cff.py still needed. It's more reasonable to delete it if it does not used any more. How to check all dependencies?

This should be agreed together with @cms-sw/generators-l2
If they can be cleaned up, that's welcome,
In any case, if the configs remain in the release, they should better get fixed.

wouf · 2022-02-09T09:18:35Z

@perrotta @cms-sw/generators-l2 I'm afraid this question would be redirected to me (as HI GEN contact). Anyway files
Configuration/Generator/python/Pyquen2013Settings_cff.py and Configuration/Generator/python/hydjet2DefaultParameters2015_cff.py does not used in any Configuration/Generator cfi config.

wouf · 2022-02-09T09:48:26Z

@perrotta One more question: I can run llvm-analyser:

scram build -f compile COMPILER=llvm-analyzer SCRAM_IGNORE_MISSING_COMPILERS=yes

but it shows only memory leak bugs. How to see all types?

perrotta · 2022-02-09T09:53:54Z

@perrotta One more question: I can run llvm-analyser:
scram build -f compile COMPILER=llvm-analyzer SCRAM_IGNORE_MISSING_COMPILERS=yes
but it shows only memory leak bugs. How to see all types?

No idea, maybe @smuzaffar can help
I would personally trust what's reported by the static analyzer in these PR tests, or in the IBs:
https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/ib/CMSSW_12_3_X

smuzaffar · 2022-02-09T10:15:30Z

@wouf , as @perrotta mentioned, llvm-analyzer reports are available on the IB page. If you run locally then you can also see the same reports in your cmssw dev directory e..g

scram p CMSSW_12_3_X_2022-02-08-2300
cd CMSSW_12_3_X_2022-02-08-2300/
cmsenv
git cms-addpkg GeneratorInterface/Hydjet2Interface
scram build compile COMPILER=llvm-analyzer SCRAM_IGNORE_MISSING_COMPILERS=yes

and reports should be available in your dev area e.g. html file under Hydjet2Hadronizer.plist

wouf · 2022-02-09T11:00:46Z

@wouf , as @perrotta mentioned, llvm-analyzer reports are available on the IB page. If you run locally then you can also see the same reports in your cmssw dev directory e..g
scram p CMSSW_12_3_X_2022-02-08-2300
cd CMSSW_12_3_X_2022-02-08-2300/
cmsenv
git cms-addpkg GeneratorInterface/Hydjet2Interface
scram build compile COMPILER=llvm-analyzer SCRAM_IGNORE_MISSING_COMPILERS=yes
and reports should be available in your dev area e.g. html file under Hydjet2Hadronizer.plist

Thank You @smuzaffar ! But there is only Memory Error, how to see also ThreadSafety, like here?

smuzaffar · 2022-02-09T11:18:06Z

I think for these static checks you need to do something like https://github.com/cms-sw/cms-bot/blob/master/pr_testing/test_multiple_prs.sh#L773-L776 e.g.

git cms-addpkg Utilities/StaticAnalyzers
export USER_CXXFLAGS='-Wno-register -DEDM_ML_DEBUG -w'
export SCRAM_IGNORE_PACKAGES='Fireworks/% Utilities/StaticAnalyzers'
export USER_LLVM_CHECKERS='-enable-checker threadsafety -enable-checker cms -enable-checker deprecated -disable-checker cms.FunctionDumper'
scram b -k -j 32 checker SCRAM_IGNORE_SUBDIRS=test

wouf · 2022-02-09T11:20:43Z

I think for these static checks you need to do something like https://github.com/cms-sw/cms-bot/blob/master/pr_testing/test_multiple_prs.sh#L773-L776 e.g.
git cms-addpkg Utilities/StaticAnalyzers
export USER_CXXFLAGS='-Wno-register -DEDM_ML_DEBUG -w'
export SCRAM_IGNORE_PACKAGES='Fireworks/% Utilities/StaticAnalyzers'
export USER_LLVM_CHECKERS='-enable-checker threadsafety -enable-checker cms -enable-checker deprecated -disable-checker cms.FunctionDumper'
scram b -k -j 32 checker SCRAM_IGNORE_SUBDIRS=test

Thank You!

wouf · 2022-02-19T13:28:55Z

@wouf , those changes are relevant to suppress those warning messages. Is that possible to open a PR, implement it on top of this PR? I can do that if you needed help.

@SiewYan yes, could You please!

@SiewYan Do You already created such PR? If not I can try to include it to the memory leak bug fix PR, I'm preparing right now.

SiewYan · 2022-02-19T16:11:45Z

@wouf , I have not, you may proceed with the fix in the PR. thank you.

Hydjet++ 2.4.3 integration

0f4ded5

cmsbuild added this to the CMSSW_12_2_X milestone Dec 1, 2021

cmsbuild added code-checks-pending generators-pending operations-pending orp-pending pdmv-pending pending-signatures tests-pending upgrade-pending labels Dec 1, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Dec 1, 2021

Code format update

9d6c36e

cmsbuild added code-checks-pending and removed code-checks-rejected labels Dec 1, 2021

wouf mentioned this pull request Dec 1, 2021

external lib for HYDJET++ cms-sw/cmsdist#7483

Merged

cmsbuild added code-checks-rejected and removed code-checks-pending labels Dec 1, 2021

Code format update2

ca83aa6

cmsbuild added code-checks-pending and removed code-checks-rejected labels Dec 1, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 1, 2021

This was referenced Dec 1, 2021

Update Run3 offline data GT and mcRun3 GTs with final DD4HEP #36326

Merged

Extending SeedingHitSet & SeedGeneratorFromProtoTracksEDProducer #36184

Merged

cmsbuild added orp-approved and removed orp-pending labels Feb 8, 2022

cmsbuild merged commit 548af36 into cms-sw:master Feb 8, 2022

wouf deleted the from-CMSSW_12_2_X_2021-12-01-1100 branch February 8, 2022 10:06

cmsbuild mentioned this pull request Feb 8, 2022

Implementation of beam halo tagger algorithm for the endcap photons #36901

Merged

wouf mentioned this pull request Feb 9, 2022

bug fixes for Hydjets and Pyquen #36918

Merged

wouf mentioned this pull request Feb 24, 2022

memory leak fixes for Hydjets #37053

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hydjet++ 2.4.3 integration #36316

Hydjet++ 2.4.3 integration #36316

wouf commented Dec 1, 2021 •

edited

Loading

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

srimanob commented Dec 4, 2021

cmsbuild commented Feb 8, 2022

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

wouf commented Feb 8, 2022

SiewYan commented Feb 8, 2022 •

edited

Loading

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

SiewYan commented Feb 9, 2022 •

edited

Loading

perrotta commented Feb 9, 2022 •

edited

Loading

wouf commented Feb 9, 2022

wouf commented Feb 9, 2022

perrotta commented Feb 9, 2022

wouf commented Feb 9, 2022

wouf commented Feb 9, 2022 •

edited

Loading

perrotta commented Feb 9, 2022

smuzaffar commented Feb 9, 2022

wouf commented Feb 9, 2022

smuzaffar commented Feb 9, 2022

wouf commented Feb 9, 2022

wouf commented Feb 19, 2022

SiewYan commented Feb 19, 2022

Hydjet++ 2.4.3 integration #36316

Hydjet++ 2.4.3 integration #36316

Conversation

wouf commented Dec 1, 2021 • edited Loading

PR description:

PR validation:

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

cmsbuild commented Dec 1, 2021

srimanob commented Dec 4, 2021

cmsbuild commented Feb 8, 2022

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

wouf commented Feb 8, 2022

SiewYan commented Feb 8, 2022 • edited Loading

perrotta commented Feb 8, 2022

wouf commented Feb 8, 2022

SiewYan commented Feb 9, 2022 • edited Loading

perrotta commented Feb 9, 2022 • edited Loading

wouf commented Feb 9, 2022

wouf commented Feb 9, 2022

perrotta commented Feb 9, 2022

wouf commented Feb 9, 2022

wouf commented Feb 9, 2022 • edited Loading

perrotta commented Feb 9, 2022

smuzaffar commented Feb 9, 2022

wouf commented Feb 9, 2022

smuzaffar commented Feb 9, 2022

wouf commented Feb 9, 2022

wouf commented Feb 19, 2022

SiewYan commented Feb 19, 2022

wouf commented Dec 1, 2021 •

edited

Loading

SiewYan commented Feb 8, 2022 •

edited

Loading

SiewYan commented Feb 9, 2022 •

edited

Loading

perrotta commented Feb 9, 2022 •

edited

Loading

wouf commented Feb 9, 2022 •

edited

Loading