Add flash match performance script - #40
Conversation
…is-asleep/spine into feature/fmatch_performance
francois-drielsma
left a comment
There was a problem hiding this comment.
Hi @bear-is-asleep, please address these minor comments and I'll merge it in!
There was a problem hiding this comment.
Could you please remove these spurious new line changes? I am guessing this is something your editor does by default? If there is a reason to operate this change repo wise it would make sense to do it globally and in a separate PR.
There was a problem hiding this comment.
Could you move this import to spine/ana/diag/__init__.py or spine/ana/metric/__init__.py depending on the answer to the question in the other comment?
There was a problem hiding this comment.
Could you move this file under either spine/ana/diag or spine/ana/metric depending on function:
diagfor diagnostics tool, i.e. are things sound in a specific setting (e.g. input track completeness)metricfor generic performance metrics on a specific reco tool (e.g. semantic accuracy)
Also, please rename it more generically as flash_match.py, for instance (its location defines its purpose) and give the class a name which fits the existing pattern of analysis tools, e.g. FlashMatchAna.
e30b776 to
e7ef639
Compare
|
Thanks for the original contribution. The package has evolved substantially since this PR was opened, and the request is now covered by the current FlashMatchingAna implementation in spine.ana.metric.optical. Commit 1227cb60 extends it with flash volume IDs and configurable extra reco/truth interaction attributes, preserving the useful performance context from this proposal without carrying forward the old detector-specific layout. Superseded by the implementation on main, so I am closing this PR. |
Returns csv that saves important flash match metrics.