Skip to content

Add flash match performance script #40

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

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

bear-is-asleep
Copy link
Contributor

Returns csv that saves important flash match metrics.

Copy link
Contributor

@francois-drielsma francois-drielsma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bear-is-asleep, please address these minor comments and I'll merge it in!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@francois-drielsma francois-drielsma Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this file under either spine/ana/diag or spine/ana/metric depending on function:

  • diag for diagnostics tool, i.e. are things sound in a specific setting (e.g. input track completeness)
  • metric for 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.

@bear-is-asleep bear-is-asleep force-pushed the feature/fmatch_performance branch from e30b776 to e7ef639 Compare March 4, 2025 23:34
@bear-is-asleep bear-is-asleep marked this pull request as draft March 6, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants