Let's go for part III: 14) in madgraph/interface/extended_cmd.py I see a misc.sprint remaining. @@ -595,20 +601,22 @@ to_rm = len(self.completion_prefix) - 1 Nbegidx = len(line.rsplit(os.path.sep, 1)[0]) + 1 data = compfunc(Ntext.replace('\ ', ' '), line, Nbegidx, endidx) + misc.sprint(data) 15) Note for the functionality review: Potentially deep change in the auto-completion behavior: Need extensive testing of all auto-completion to check if they still work. (use the opportunity to add unitest on the auto-completion) - # correct wrong splitting with '-' - elif line and line[begidx-1] == '-': + # correct wrong splitting with '-'/"=" + elif line and line[begidx-1] in ['-',"=",':']: 16) in extended_cmd, a new parameter self.case is added to the question interfase/ the name is not clear enough if the question is then case sensitive or case insensitive. Therefore the name has to be modified. 17) in madgraph/interface/launch_ext_program.py @@ -766,7 +766,7 @@ can you check the following change: (the print is kept but not the compilation) # Make pythia8 print "Running make for pythia8 directory" - misc.compile(cwd=os.path.join(self.running_dir, os.path.pardir), mode='cpp') + #misc.compile(cwd=os.path.join(self.running_dir, os.path.pardir), mode='cpp') 18) in madevent_interface for the check_pythia8 command 18.1) you have the line + # If not pythia-pgs path 18.2) you have + mode = laststep[0].split('=')[-1] + if mode not in ['auto', 'pythia', 'delphes']: should not you also support laststep=pythia8 18.3) The following line are not bugged (output_file not define when untarring: + input_file = pjoin(self.me_dir,'Events',self.run_name, 'unweighted_events.lhe') + #output_file = pjoin(self.me_dir, 'Events', 'unweighted_events.lhe.gz') + if not os.path.exists('%s.gz'%input_file): + if os.path.exists(input_file): + misc.gzip(input_file, keep=True, stdout=output_file) + else: + raise self.InvalidCmd('No event file corresponding to %s run. ' + % self.run_name) 19) complete_shower This use a useless (and dangerous eval) + elif len(args)>1 and args[1] in self._interfaced_showers: + return eval("self.complete_%s(text,'%s',%d,%d)"% + (args[1],line.replace(args[0]+' ',''),begidx-len(args[0])-1, + endidx-len(args[0])-1)) this should be replaced by a nicer (splitting in multiple will also be more readable) return getattr(self, 'complete_%s' % text)(text, args[1],line.replace(args[0]+' ',''), begidx-len(args[0])-1, endidx-len(args[0])-1)) 20) in complete_pythia8: apply also the replacement for glob.glob in this file/function. (if not done already) 21) in setup_pythia8 Not sure to understand this: + # Use defaultSet not to overwrite the current userSet status + PY8_Card.defaultSet('HEPMCoutput:file','PY8_hepmc.fifo') This line never do anything right (the parameter will always be set by the user at this stage. Do I miss a point? 22)in setup_pythia8 elif PY8_Card['HEPMCoutput:file'] in ['','/dev/null']: Should not we also add None in this list (this is my python bias) 23) # Normalize the relative path if given as relative by the user. HepMC_event_output = pjoin(self.me_dir,'Events', self.run_name, PY8_Card['HEPMCoutput:file']) In principle all relative path should be understand from the current work directory. If you want to keep it this way. please specify it clearly in the py8card. 24) the qcutList parameter of the PY8 is ... not a list (just a string). Would not make it more sense to have it as a list (will ensure a better/more uniform parsing) (same for SysCalc:tmsList) 25) in setup_pythia8 if PY8_Card['Merging:Process']=='': raise self.InvalidCmd('When running CKKWl merging, the user must'+ " specifiy the option 'Merging:Process' in pythia8_card.dat") a weblink to documetation would be nice here. 26) in do_pythia8, I see this: elif int(self.run_card['ickkw'])==2 or \ self.run_card['ktdurham']>0.0 or self.run_card['ptlund']>0.0: run_type = 'CKKW' Do we really want to support the option self.run_card['ickkw'] 27) In case of fifo, you always run PY8 on local not via the cluster. Is this really mandatory? 28) If you run in cluster mode, you do not specify the input/output file, this will make the condor cluster really/really slow. Would be better to ship the lhe (zipped) and return the hepmc (zipped) and do not have constant reading/writting on the central disk 29) in the question for the type of run: + elif answer == 'madanalysis5': + switch_assign('madanalysis5', valid_options['madanalysis5'][-1]) + switch_assign('detector', 'OFF') This does not make sense to me. This is all section is for retro-compatibility purpose. What is the point to add an unclear syntax here. I would just remove that option/mode. 30) I'm surprised by: @@ -5313,7 +5364,7 @@ misc.detect_cpp_std_lib_dependence(self.options['cpp_compiler'])) else: compiler_options.append('--cpp_standard_lib=%s'% - misc.detect_cpp_std_lib_dependence(self.options['cpp_compiler'])) + misc.detect_cpp_std_lib_dependence('g++')) this does not sound a very good idea. I guess we should think more about this (the problem is obviously to detect correctly clang) 31) in madgraph/iolibs/export_cpp.py if self.version =="8.2": replace_dict['include_prefix'] = 'Pythia8/' else: + raise Exception replace_dict['include_prefix'] = '' This sounds as a wrong change 32) I'm worried about the following change: (in export_fks) @@ -732,10 +733,6 @@ makejpg = False else: makejpg = True - if 'online' in flaglist: - online = True - else: - online = False 33) another file to fix for the glob.glob: gen_crossxhtml 34) I'm not sure to understand the html/database strateagy for madanalysis5_hadron. it's often written that this is a placeholder for 'done' but then I see this... I'm confused: + elif ttype in ['madanalysis5_hadron']: + # We can use the same template as pgs here + template = sub_part_template_pgs + local_dico['type'] = 'hadron MA5' + # Nothing else needs to be done for now, since only type and + # run_mode must be defined in local_dict and this has already + # been done. so you just have an empty line? Thats' it for this morning. Continuing soon current status: line 8917 (5%)