icecube / skywriter

Upstream Tools for SkyDriver & the Skymap Scanner
MIT License
0 stars 0 forks source link

Improve handling of multiple subevents and particle extraction #15

Closed mlincett closed 11 months ago

mlincett commented 11 months ago

This PR fixes #14 with a more graceful handling of multiple subevents produced by I3TriggerSplitter.

The core changes are:

  1. the relevant I3 modules are now run only for subevents where the uid matches the one in the original P-frame;
  2. almost no logic is run on P frames that are meant to be skipped, hence the exception handling has been largely simplified;
  3. the custom extractor for "extra particles" is removed: extra keys are retrieved from the original P-frame (as suggested by @tianluyuan before).

Bonus changes:

ric-evans commented 11 months ago

Looking forward, we'll need to define an entry point pattern. I see two equally suitable patterns:

  1. multiple entry points: each module has an if-main block
    • multiple argparses
  2. one entry point: make a __main__.py
    • one argparse
mlincett commented 11 months ago

This looks good. Just to clarify is the idea that I3Particles from the original p-frame would still be user specified via --extra, including those needed in the online filtering? Or are some keys in the original p-frame always saved by default? Maybe a README would be nice to add to explain the behavior.

All the keys required for the realtime processing are copied from the input to the output P-frame.

The coordinates of I3Particles specified in the --extra keys are extracted and written in plaintext to the JSON, but there is no warranty that the corresponding data will be in the output I3Frame (as the user could choose a key not among the ones that are routinely copied). This is no different from the current main branch behavior, I believe (but of course it can be improved in the future).

I agree this can be clarified in a README.

mlincett commented 11 months ago

Looking forward, we'll need to define an entry point pattern. I see two equally suitable patterns:

1. multiple entry points: each module has an `if-main` block

   * multiple `argparse`s

2. one entry point: make a `__main__.py`

   * one `argparse`

We don't really have actual multiple modules now, do we?

I mostly use the i3_to_json.py as a script for testing but then I use i3_to_json as a function in SkyMist for all practical purposes. So no strong preference in either direction.

ric-evans commented 11 months ago

Looking forward, we'll need to define an entry point pattern. I see two equally suitable patterns:

1. multiple entry points: each module has an `if-main` block

   * multiple `argparse`s

2. one entry point: make a `__main__.py`

   * one `argparse`

We don't really have actual multiple modules now, do we?

I mostly use the i3_to_json.py as a script for testing but then I use i3_to_json as a function in SkyMist for all practical purposes. So no strong preference in either direction.

You're right, we don't right now. However, if you wanted to go with one entry point, then there'd need to be refactoring that would be easier to do now than later when we add a new script. BUT, it sounds like multiple entry points are fine :smile: we'll just have to use python -m so the helper module is available