JeffersonLab / jlab_datascience_exp_hall

MIT License
0 stars 1 forks source link

Implement Hall B dataparser for Aidapt #1

Closed dlersch closed 1 month ago

dlersch commented 9 months ago

Goal is to migrate the AIDAPT code from the current repo to this repo. Each hall has its own folder: HallA, HallB, HallC and HallD. For now, please add the HallB dataparser for aidapt. The dataparser should inherit from the jlab_datascience_core

sgoldenCS commented 9 months ago

Data parser for AIDAPT has been uploaded and inherits from the jlab_datascience_core. A few functions have been overwritten to implement the interface but have trivial implementations (pass). Specifically:

dlersch commented 9 months ago

This looks really good! Here are few thoughts that we can discuss:

  1. The .forward() function is something we invented for SciDAC. It has no proper meaning somewhere else. I would just remove it and have the load_data() function.
  2. I noticed that the transformation from lab variables to invariant system is included into the parser. I full understand the logic behind it. But in order to adhere to the simplicity of the workflow, the functions lab_to_inv / lab_to_com should be in the data preparation module. If you take out these functions, then you have a simple numpy loader, which can be re-used for any task in Aidapt and thus makes it more a powerful tool
  3. Please add a unit-test. I know these are annoying, but they do make your live easier in the long-run. You need to convince yourself that the code is exactly doing what it is supposed to do.