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

Adding new interface forTauola 114 7 2 x #4208

Closed
wants to merge 4 commits into from
Closed

Adding new interface forTauola 114 7 2 x #4208

wants to merge 4 commits into from

Conversation

inugent
Copy link
Contributor

@inugent inugent commented Jun 11, 2014

This pull request has the update to use the new features in Tauola 1.1.4 and MC-Tester for validation. Contains patch for RelVal crash in 7_1_X.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @inugent for CMSSW_7_2_X.

Adding new interface forTauola 114 7 2 x

It involves the following packages:

GeneratorInterface/ExternalDecays
GeneratorInterface/MCTESTER
GeneratorInterface/TauolaInterface

The following packages do not have a category, yet:

GeneratorInterface/MCTESTER

@vciulli, @nclopezo, @thuer, @cmsbuild, @bendavid, @Degano, @ktf can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@vciulli
Copy link
Contributor

vciulli commented Jun 11, 2014

+1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@bendavid
Copy link
Contributor

Right, these are the interface changes that go along with it so this should indeed go in.

@davidlange6
Copy link
Contributor

sorry for my slow reaction about this PR - I'm not convinced we should distribute MCTESTER within CMSSW. Is there a particular need to do so? (are we changing it somehow?) In this PR it appears to be a completely standalone thing.

In any case, we won't want the pdf/tex/ps.gz stuff...

@inugent
Copy link
Contributor Author

inugent commented Jun 24, 2014

Hi David (adding the Gen Conveners and other people who may be
interested in the discussion),
I think there are arguments on both sides, but I was not included
in any discussion on this topic. The main arguments I know of are:
MCTester is useful for validating generator against benchmarks produced
by the authors of the generator, but requires some work to adapt it to
CMSSW (ie a debuged interface and a modified version for CMSSW to avoid
conflict with the CMSSW data format - however the modified version is
external to CMSSW). However, as you point out it is a generator specific
tool with limited use (only for defining/validating benchmarks in
CMSSW). It will not be used in a central production and requires
additional work after running on the dataset. Previously, it was in
CMSSW, and then it was dropped when moving to git from CMSSW but not
cmsdist (I was not included in any discussion and there was a history
with buggy versions before I joined CMSSW. When I brought it up in the
Generator meeting there was no objection to adding it back into CMSSW).
I am not sure what the best option is. How would you recommend we store
this tool (Note: it is already in cmsdist so we may also want to include
this in the discussion)? Thank you and best regards,
Ian

On 2014-06-23 9:37 AM, David Lange wrote:

sorry for my slow reaction about this PR - I'm not convinced we should
distribute MCTESTER within CMSSW. Is there a particular need to do so?
(are we changing it somehow?) In this PR it appears to be a completely
standalone thing.

In any case, we won't want the pdf/tex/ps.gz stuff...


Reply to this email directly or view it on GitHub
#4208 (comment).

Dr. Ian M. Nugent
III. Physikalisches Institut B
RWTH Aachen, D-52056 Aachen
Tel: +49 241 80-27295
Email: nugent@physik.rwth-aachen.de

@bendavid
Copy link
Contributor

Hi Ian,
Sorry I'm a bit confused now. If MCTESTER is already in cmsdist as an external, why does it need to be in the CMSSW repository?

What is really needed in CMSSW on top of the external?

@inugent
Copy link
Contributor Author

inugent commented Jun 26, 2014

Hi Josh,
There are two part here: 1) the interface is being added to CMSSW. 2) The analyze directory with some examples is being added because you need to be able run this section of code and you do not have write permission in the built cmsdist libraries where is is already built (The current solution is have it in CMSSW.git with minor modification to compile and link this code and then run in CMSSW).
Ian

@bendavid
Copy link
Contributor

Why is write permission needed to run the code?

@nclopezo nclopezo modified the milestones: CMSSW_7_2_0_pre2, CMSSW_7_2_0_pre1 Jun 27, 2014
@inugent
Copy link
Contributor Author

inugent commented Jun 27, 2014

Hi Josh,
Because root files are analysed in the analysis directory my the
MCTester code. This generates additional files, such as pdf's which are
created inside the analysis directory. This is how the authors
constructed MC-Tester. Best regards,
Ian

On 2014-06-26 11:28 PM, Josh Bendavid wrote:

Why is write permission needed to run the code?


Reply to this email directly or view it on GitHub
#4208 (comment).

Dr. Ian M. Nugent
III. Physikalisches Institut B
RWTH Aachen, D-52056 Aachen
Tel: +49 241 80-27295
Email: nugent@physik.rwth-aachen.de

@ghost
Copy link

ghost commented Jul 10, 2014

@inugent This Pull Request cannot be merged. Please rebase.

@bendavid
Copy link
Contributor

Hi @inugent is it correct that this pull request is superseded by #4707 and should be closed?

@inugent
Copy link
Contributor Author

inugent commented Jul 23, 2014

Yes, this can be closed.

@inugent inugent closed this Jul 23, 2014
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

6 participants