LDMX-Software / ldmx-sw

The Light Dark Matter eXperiment simulation and reconstruction framework.
https://ldmx-software.github.io
GNU General Public License v3.0
21 stars 19 forks source link

Expand python modules with more helpful/robust functions #780

Closed tomeichlersmith closed 4 years ago

tomeichlersmith commented 4 years ago

Our python modules currently are very bare-boned, and I think we can expand them to make the use of ldmx-sw much easier and robust. I am planning to add more interfaces that will make the python configuration less likely to make small mistakes.

Here is a list of planned improvements:

Isn't this just more code nonsense? Yes, but these changes will make ldmx-sw easier to use for newbies and help it be more robust for all of us. Hopefully will drastically reduce the number of small errors like mispelling parameter names.

tomeichlersmith commented 4 years ago

Two updates on my thought process:

  1. Boost.Python looks like a good wrapper for the python-C interface that I am going to use.
  2. I am going to try to rid ourselves of the parameters python members. Instead, each object that needs parameters will have a "mirror" python object that has configuration parameters. Then the ConfigurePython class can import thess members into a C++ map that the class will have access to. This way, the python configuration object will check for misspellings at runtime.
omar-moreno commented 4 years ago

Before deciding on a library please make sure to consider all alternatives e.g. https://github.com/pybind/pybind11.

Also, this is a big change so it would be good to see a presentation on this before moving forward.

On Tue, Jun 16, 2020, 1:48 PM Tom Eichlersmith notifications@github.com wrote:

Two updates on my thought process:

  1. Boost.Python https://www.boost.org/doc/libs/1_69_0/libs/python/doc/html/tutorial/tutorial/embedding.html looks like a good wrapper for the python-C interface that I am going to use.
  2. I am going to try to rid ourselves of the parameters python members. Instead, each object that needs parameters will have a "mirror" python object that has configuration parameters. Then the ConfigurePython class can import thess members into a C++ map that the class will have access to. This way, the python configuration object will check for misspellings at runtime.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LDMX-Software/ldmx-sw/issues/780#issuecomment-645003822, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4JMXFPZRXTJ7LBH7DEROLRW7LDHANCNFSM4NKUX5FA .

tomeichlersmith commented 4 years ago

@omar-moreno Two things:

  1. I am going to break Python2 compatibility with these changes for the same reason I reference in PR #806 .
  2. I am going to break configuration backwards compatibility. This means that you can still read in event files before these changes, but you will need to update your run scripts. I know this is a hassle, but this is the way we should be going and honestly it's a lot of work to try to maintain backwards compatibility.
omar-moreno commented 4 years ago

@omar-moreno Two things:

1. I am going to break Python2 compatibility with these changes for the same reason I reference in PR #806 .

Sounds good.

2. I am going to break _configuration_ backwards compatibility. This means that you can still read in event files before these changes, but you will need to update your run scripts. I know this is a hassle, but this is the way we should be going and honestly it's a lot of work to try to maintain backwards compatibility.

Just to be clear, this should only impact the python configs right? This has (or should) have nothing to do with the event model?

tomeichlersmith commented 4 years ago

Correct, I am not touching the event model at all. The only C++ class that I am changing is ConfigurePython.