Skip to content
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

Filter mask #597

Merged
merged 18 commits into from
Oct 11, 2023
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/graphnet/data/dataconverter.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def __init__(
workers: int = 1,
index_column: str = "event_no",
icetray_verbose: int = 0,
I3filters: List[str] = [],
):
"""Construct DataConverter.

Expand Down Expand Up @@ -166,6 +167,7 @@ def __init__(
self._sequential_batch_pattern = sequential_batch_pattern
self._input_file_batch_pattern = input_file_batch_pattern
self._workers = workers
self._I3filters = I3filters

# Create I3Extractors
self._extractors = I3ExtractorCollection(*extractors)
Expand Down Expand Up @@ -435,6 +437,8 @@ def _extract_data(self, fileset: FileSet) -> List[OrderedDict]:
continue
if self._skip_frame(frame):
continue
if self._filter_mask(frame, self._I3filters):
Copy link
Collaborator

@RasmusOrsoe RasmusOrsoe Sep 25, 2023

Choose a reason for hiding this comment

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

self._filter_mask will return False by default if "FilterMask" not in frame. That means that everyone will be unable to extract i3frames without this field. That's not good!

You can resolve this by either

A) Make _filter_mask return True by default in these cases
B) change if self._filter_mask(frame, self._I3filters) to if self._filter_mask(frame, self._I3filters) & len(self._I3filters) > 0. This way, the frames are only skipped if the user wants to filter out frames this way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should very much work such that a True return from the _filter_mask call will skip the frame (through a continue call) while a False return will move on to include the frame. I agree this is a little counter intuitive and should probably be specified in the description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry. I was fooled. You are right!

continue

# Try to extract data from I3Frame
results = self._extractors(frame)
Expand Down Expand Up @@ -566,3 +570,27 @@ def _skip_frame(self, frame: "icetray.I3Frame") -> bool:
if frame["I3EventHeader"].sub_event_stream == "NullSplit":
return True
return False

def _filter_mask(
self, frame: "icetray.I3Frame", I3filters: List[str]
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can get rid of the codeclimate error if you let _filter_mask access the i3filters inside the function, instead of passing them as argument.

) -> bool:
"""Check if specified condition(s) are met.

Args:
frame: I3Frame to check.
I3filters: List of I3Filters to check for pass.
"""
if "FilterMask" not in frame:
self.warning_once(
"FilterMask not found in frame. Skipping filter checks."
)
return False
for filter in I3filters:
if filter not in frame["FilterMask"]:
self.warning_once(
f"Filter {filter} not found in frame. Skipping."
)
continue
if frame["FilterMask"][filter].condition_passed is False:
return True
return False