Hi Johan, Thanks a lot for this review. I skip all the points that you have raised and which didn't require discussion. > However, I don't like the way it's done. To begin with, this should only be added for particles that have a decay length > (say) some fraction of a mm or perhaps a micrometer to be safe, since otherwise it's not detectable anyway. Also, this should not be a command but rather a setting: "set add_time_of_flight True". It should then be done automatically on the unweighted_events.lhe file before running plots or Pythia. We can also imagine the threshold to be set using the set command, although I don't think that really matters (if the user wants this, there is probably some particle with a considerable decay length, and usually all other particles will have very short decay lengths anyway). In fact, I don't really care about this function, the only point to have add is 1) this is times to times ask to have it. 2) One user had a lot of trouble to implement a simple code for doing this and was sending me mail after mail. So I figure that it will be faster for me to implement a simple command that answering all his question. This is why, I didn't put too much effort to fully interface it with the rest of the code and things like that. But if you want to do it, feel free to do so. The idea of having a command allows to be able to run it on external lhe files (assuming that this is the same model). (and that it was faster to implement) > 433 @@ -896,11 +920,18 @@ > ... > 444 + restrict_file = None > 445 + if os.path.exists(pjoin(ufo_path, 'restrict_default.dat')): > 446 + restrict_file = pjoin(ufo_path, 'restrict_default.dat') > 447 + model = import_ufo.import_model(modelname, decay=True, > 448 + restrict_file=restrict_file) > > I'm not so clear about this change. What happens if some other restriction is used? The modelname will be (say) sm-lepton_masses, but if restrict_file is set to restrict_default, will that take precedence? In such case, they shouldn't have conflicts, but it happens that the name of the model is simply sm while a restriction is suppose to be used. This is the point of those lines. This happens for examples when you are using the customize_model options. > === modified file 'madgraph/interface/madgraph_interface.py' > ... > 578 + > 579 + if args[0] == 'Delphes': > 580 + data = open(pjoin(MG5DIR, 'Delphes','data','DetectorCard.dat')).read() > 581 + data = data.replace('data/', 'DELPHESDIR/data/') > 582 + out = open(pjoin('Template', 'Cards', 'delphes_card_default.dat'), 'w') > 583 + out.write(data) > 584 + > > What happened to the trigger card? Since this one is not modified and that Delphes2 is not develop anymore, I don't think that this is a problem. > Note that this will NOT work with Delphes 3, where the card is called examples/delphes_card_CMS.tcl (and there is no trigger_card). I would suggest that you implement the copy for Delphes 3, as well as the change from run_delphes to run_delphes3 in that case, already now, so we don't forget it later. Done (and an install Delphes2/Delphes3) > === modified file 'madgraph/iolibs/export_cpp.py' > ... > 607 + curr_final_id = [l.get('id') for l in \ > 608 + proc.get('legs') if l.get('state')] > 609 + for id in decay_id: > 610 + curr_final_id.remove(id) > > Actually I don't think this works. I think it has to be > curr_final_id = [l.get('id') for l in \ > proc.get('legs') if l.get('state') and \ > l.get('id') not in decay_id] > otherwise you will still leave one if you have multiple identical particles (such as p p > z z, z > ...). Note same comment a little further down: > 632 + for id in decay_id: > 633 + curr_id.remove(id) In fact this is fine sine decay_id will have more than one the z pdg_code in that case. Also this is for the cpp output so if I'm correct such scenario can't happen. But your code is nicer, so I have done the modification. > The LHE parser is awesome! However, it's not clear how it will deal with the information in the 1.6.0 branch, where there is a lot of info after the event information. I would suggest to simply store anything that comes after the end of the event info (the momenta etc) in a single multiline string, and output it inchanged. Otherwise you will have a LOT of trouble when we add more tags. In fact all information include after (and including) a line containing the string "<" is for the moment assumed to be part of the reweight part and is therefore keep unchanged. So this is exactly what you ask for. But I would like to have that part correctly parse at a point. Since I would be nice to be able to use this parser to read / add weight. In fact I will need that at a point for the reweighing tools. Thanks again for the review. Olivier On Feb 25, 2013, at 10:40 PM, Johan Alwall