Offset-wise event types

[JBernete]

The event-type partitioning can be now calculated for every given log_reco_energy AND camera_offset bin. Use 'scripts/export_event_types_2.py' for this. The training now can take multiple offset reduced data files simultaneously.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[TarekHC]

Hi Juan,

Before doing a full review, let me give you a bit of fast feedback and ask a couple of questions:

I would not call scripts/functions "xxxx_2". Please improve the naming. For example "partition_event_types_2" -> "partition_event_types_vs_offset". Also export_event_types_2 -> export_event_types_vs_offset

Would it be possible to add a flag to the current "partition_event_types" function, instead of creating a new one? Do you 100% need a new function? I understand that for tests it is useful to create a new one. But for simplicity, I would expand the one we have now, and not create a completely new one.

I will check in more detail next week.

Tarek

[orelgueta]

Apart from @TarekHC's comments, which I agree with, there are a couple of other reasons to wait a few days with this PR. First, since now there are few developers and the majority of the changes in this PR are style changes, I added a pre-commit hook to avoid such things. Please wait with this PR, add the pre-hook which I will add in a different PR and then we review this one. I think it will make it easier to go through then.

Other than that, I implemented the reading of multiple datasets slightly differently (but took something from your implementation). It's very simple and similar, but I wanted to keep the "API" the same so I did it this way. Maybe you could use my implementation instead?

I will open PRs soon with both these changes so you can see what I mean and let me know if you agree with them.

[orelgueta]

Now that the other PRs are merged (thanks for the reviews!), I guess you should merge the main to your code first and then implement the changes Tarek mentioned.

[JBernete]

I think it's ready now.

I could merge the scripts 'export_event_types.py' and 'export_event_types_2.py', so now choosing between offset-wise or not is a tag in that script.

I didn't do the same for the functions 'partition_event_types' and 'partition_event_types_2' because that would be more complicated and I would need to almost duplicate the entire function filling it with ifs. So I think it's easier to understand with two different functions and I changed the name of the second one to 'partition_event_types_offsetwise'.

[JBernete]

All these changes now work. Please have a look and let me know if I should change anything else.

[orelgueta]

All of my comments were addressed now so I am merging this. The discussion with @TarekHC about the offset binning and statistics can continue here or in a dedicated issue.