Splitmerge #28
Splitmerge #28
Conversation
…photon, neutral and charged hadrons
There was a problem hiding this comment.
Hi again Alice,
This is excellent. In addition to your new features I appreciate the effort to clean things up.
My comments are really mostly about cosmetics. I only request a few changes before approval.
I guess the main thing is : again, what did we decide with pfinput.py? are you going to remove it later?
|
|
||
| class PDebugger(Analyzer): | ||
| ''' Module to output physics debug output | ||
| ''' |
There was a problem hiding this comment.
Could you please add documentation for this module?
the documentation should include an example configuration fragment, as done in other analyzers, e.g.
https://github.com/cbernet/heppy/blob/master/analyzers/Filter.py
|
|
||
| #turn on output to stdout if requested | ||
| if hasattr(self.cfg_ana, 'output_to_stdout') and self.cfg_ana.output_to_stdout: | ||
| pdebugging.set_stream(sys.stdout,level=logging.INFO) |
There was a problem hiding this comment.
the log level could be passed through cfg_ana
There was a problem hiding this comment.
Can we discuss?
| pdebugging.pdebugger.setLevel(logging.INFO) | ||
| pass | ||
|
|
||
| def process(self, event): |
There was a problem hiding this comment.
not clear what this analyzer is doing. from the process method, i see that it only prints the event number, but the docstrings seem to imply that it's much more than that.
The documentation should explain why this analyzer is necessary.
For instance, mention that this analyzer:
- sets up the pdebugging tool, a separate logger for physics
- logs for each event the number of event
Then, explain that the pdebugging module should be used wherever the user wants a physics debug output.
no need to repeat the pdebugging documentation, the documentation for this module should be therein
| if hasattr(self.cfg_ana, 'debug_filename'): | ||
| pdebugging.set_file(self.cfg_ana.debug_filename) | ||
| pdebugging.pdebugger.setLevel(logging.INFO) | ||
| pass |
There was a problem hiding this comment.
please review the whole code and remove all unecessary pass statements
|
|
||
| def process(self, event): | ||
| pdebugging.pdebugger.info(str('Event: {}'.format(event.iEv))) | ||
| pass |
There was a problem hiding this comment.
unecessary .. last mention of it
|
|
||
| # these lines moved earlier in order to match cpp logic | ||
| if ptc.q()!=0: | ||
| pdebugger.info("Made " + ptc.track.__str__()) |
There was a problem hiding this comment.
string concatenation with + is inefficient, please use join.
| self.propagate(ptc) | ||
| if ptc.q()!=0: | ||
| pdebugger.info("Made " + ptc.track.__str__()) |
There was a problem hiding this comment.
string concatenation with + is inefficient, please use join.
| ecal = self.detector.elements['ecal'] | ||
| self.prop_helix.propagate_one(ptc, | ||
| ecal.volume.inner, | ||
| self.detector.elements['field'].magnitude ) | ||
| if ptc.q()!=0: | ||
| pdebugger.info("Made " + ptc.track.__str__()) |
There was a problem hiding this comment.
string concatenation with + is inefficient, please use join.
| for gen_ptc in ptcs: | ||
| ptc = pfsimparticle(gen_ptc) | ||
| if ptc.pdgid() == 22: | ||
| self.simulate_photon(ptc) | ||
| elif abs(ptc.pdgid()) == 11: | ||
| elif abs(ptc.pdgid()) == 11: #check with colin |
There was a problem hiding this comment.
what to check?
maybe remove these comments (with the one below for muons)
There was a problem hiding this comment.
My last code contained smear_electron(ptc) instead of propagate electron. Similarly for Muons. Was wanting to understand why the change and what implications (if any) it has.
| @@ -42,7 +42,7 @@ def __str__(self): | |||
| q = self.q(), | |||
| p4 = super(Particle, self).__str__() | |||
| ) | |||
|
|
|||
|
Hi Colin |
Emu in papas
Hi Colin
Here we go. Ran the tests and I think it should be OK.
I figured out this morning worry (had wrong number of events in test) so everything now matches to my satisfaction.
Let me know what you think.
Alice
PS your new tutorial test is super cool (had a good look it in trying to understand the random seed problems)