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

bugfix for sherpa compiled with openmpi support #20401

Merged
merged 2 commits into from Sep 18, 2017

Conversation

pmillet
Copy link
Contributor

@pmillet pmillet commented Sep 6, 2017

No description provided.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

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

It involves the following packages:

GeneratorInterface/SherpaInterface

@cmsbuild, @efeyazgan, @perrozzi, @thuer, @govoni can you please review it and eventually sign? Thanks.
@alberto-sanchez, @agrohsje, @mkirsano, @thuer this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20401/525

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20401/525/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20401/525/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly (this will soon be required for PRs to be considered)

@perrozzi
Copy link
Contributor

perrozzi commented Sep 6, 2017

@pmillet please apply the suggested patch https://cmssdt.cern.ch/SDT/code-checks/PR-20401/525/git-diff.patch and commit/push again

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

Pull request #20401 was updated. @cmsbuild, @efeyazgan, @perrozzi, @thuer, @govoni can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20401/526

@perrozzi
Copy link
Contributor

perrozzi commented Sep 6, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

-1

Tested at: 366c7b8

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
4c9605a
90a4be0
bfa71fa
3043808
72fcaf9
c8d9643
6aaa765
d0cc21f
aa14a75
484eb14
505e1cc
88641d2
b9e4992
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22750/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22750/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22750/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

  • Build:

I found an error when building:

/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Pointer/RCPtr.h: In instantiation of 'ThePEG::Pointer::RCPtr& ThePEG::Pointer::RCPtr::create(ThePEG::Pointer::RCPtr::const_reference) [with T = ThePEG::LesHouchesInterface; ThePEG::Pointer::RCPtr::const_reference = const ThePEG::LesHouchesInterface&]':
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Pointer/RCPtr.h:126:22:   required from 'static ThePEG::Pointer::RCPtr ThePEG::Pointer::RCPtr::Create(ThePEG::Pointer::RCPtr::const_reference) [with T = ThePEG::LesHouchesInterface; ThePEG::Pointer::RCPtr::const_reference = const ThePEG::LesHouchesInterface&]'
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Pointer/RCPtr.h:869:69:   required from 'static ThePEG::Pointer::PtrTraits >::pointer ThePEG::Pointer::PtrTraits >::create(ThePEG::Pointer::PtrTraits >::const_reference) [with T = ThePEG::LesHouchesInterface; ThePEG::Pointer::PtrTraits >::pointer = ThePEG::Pointer::RCPtr; ThePEG::Pointer::PtrTraits >::const_reference = const ThePEG::LesHouchesInterface&]'
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Pointer/PtrTraits.h:204:55:   required from 'typename ThePEG::Pointer::Ptr::pointer ThePEG::Pointer::new_ptr(const T&) [with T = ThePEG::LesHouchesInterface; typename ThePEG::Pointer::Ptr::pointer = ThePEG::Pointer::RCPtr]'
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-09-05-2300/src/GeneratorInterface/ThePEGInterface/plugins/ThePEGLesHouchesInterface.cc:38:53:   required from here
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Pointer/RCPtr.h:147:11: error: invalid new-expression of abstract class type 'ThePEG::LesHouchesInterface'
     ptr = new T(t);
           ^~~~~~~~
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-09-05-2300/src/GeneratorInterface/ThePEGInterface/plugins/ThePEGLesHouchesInterface.cc:30:7: note:   because the following virtual functions are pure within 'ThePEG::LesHouchesInterface':
 class LesHouchesInterface : public LesHouchesReader {
       ^~~~~~~~~~~~~~~~~~~

  • Clang:

I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 16 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-09-05-2300/src/DataFormats/Common/src/PtrVectorBase.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-09-05-2300/src/DataFormats/Common/src/ThinnedAssociation.cc 
In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_3_X_2017-09-05-2300/src/GeneratorInterface/ThePEGInterface/src/ThePEGInterface.cc:22:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Repository/Repository.h:16:
In file included from /cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/PDT/ParticleData.h:19:
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Utilities/Selector.h:131:50: error: ISO C++1z does not allow dynamic exception specifications [-Wdynamic-exception-spec]
  T & select(double rnd, double * remainder = 0) throw(range_error);
                                                 ^~~~~~~~~~~~~~~~~~
/cvmfs/cms-ib.cern.ch/nweek-02488/slc6_amd64_gcc630/external/thepeg/2.0.4-ghjeda4/include/ThePEG/Utilities/Selector.h:131:50: note: use 'noexcept(false)' instead
  T & select(double rnd, double * remainder = 0) throw(range_error);
                                                 ^~~~~~~~~~~~~~~~~~


The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
4c9605a
90a4be0
bfa71fa
3043808
72fcaf9
c8d9643
6aaa765
d0cc21f
aa14a75
484eb14
505e1cc
88641d2
b9e4992
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22750/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22750/git-merge-result

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 14, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22966/console Started: 2017/09/14 09:59

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 14, 2017

+1

Tested at: 366c7b8
Edited by @smuzaffar: As expected, workflow 534.0 failed due to gridpacks GCC ABI incompatibility

Library_Loader::LoadLibrary(): /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/runTheMatrix-results/534.0_sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+HARVESTGEN/Process/Amegic/lib/libProc_fsrchannels2.so: undefined symbol: _ZN6PHASIC14Single_Channel4NameEv
Library_Loader::LoadLibrary(): Failed to load library 'libProc_fsrchannels2.so'.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
3f63597
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22966/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22966/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22966/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
534.0 step1

runTheMatrix-results/534.0_sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+HARVESTGEN/step1_sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+HARVESTGEN.log

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
3f63597
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22966/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20401/22966/git-merge-result

@cmsbuild
Copy link
Contributor

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@pmillet
Copy link
Contributor Author

pmillet commented Sep 14, 2017

So if I read the log correctly we are back to the gcc incompatibility error as expected. I will update the relval samples for gcc630.

Library_Loader::LoadLibrary(): /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/runTheMatrix-results/534.0_sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+sherpa_ZtoEE_0j_OpenLoops_13TeV_MASTER+HARVESTGEN/Process/Amegic/lib/libProc_fsrchannels2.so: undefined symbol: _ZN6PHASIC14Single_Channel4NameEv
Library_Loader::LoadLibrary(): Failed to load library 'libProc_fsrchannels2.so'.

@pmillet
Copy link
Contributor Author

pmillet commented Sep 15, 2017

@efeyazgan, @perrozzi, @thuer, @govoni @davidlange6 - anything preventing this from being included? its needed for Sherpa to work with cms-sw/cmsdist#3405 (merged)

@perrozzi
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2)

@perrozzi
Copy link
Contributor

I was actually waiting for #20515 to be merged in order to re-run the tests. From the pure GEN point of view this is good to go, approved.

@@ -66,7 +69,7 @@ class SherpaHadronizer : public gen::BaseHadronizer {
edm::ParameterSet SherpaParameterSet;
unsigned int maxEventsToPrint;
std::vector<std::string> arguments;
SHERPA::Sherpa Generator;
SHERPA::Sherpa *Generator = new SHERPA::Sherpa();
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @pmillet - please make this a smart pointer.

isInitialized=false;
//initialization of Sherpa moved to initializeForInternalPartons
}

SherpaHadronizer::~SherpaHadronizer()
{
Generator->~Sherpa();
#ifdef USING__MPI
MPI::Finalize();
Copy link
Contributor

Choose a reason for hiding this comment

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

so this is a sign that you can not use the default CMSSW builds.. have you considered run time switches to provide this same functionality?

Copy link
Contributor Author

@pmillet pmillet Sep 15, 2017

Choose a reason for hiding this comment

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

@davidlange6 -
sherpa is either compiled with or without mpi support, so runtime switches are not an option

if it is compiled with mpi support it needs the mpi init and finalize calls even if it is not
parallelized using mpi (this is the case here)

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 3f53835 into cms-sw:master Sep 18, 2017
@pmillet pmillet mentioned this pull request Sep 18, 2017
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

5 participants