oslocyclotronlab / ompy

A python implementation of the Oslo method
https://ompy.readthedocs.io
GNU General Public License v3.0
6 stars 7 forks source link

Make ompy more modular #195

Open vetlewi opened 3 years ago

vetlewi commented 3 years ago

At some point we should consider separating the physics and the "core" classes into different modules. One way could be to have an ompy.core module that contains the Matrix, Vector classes and misc. library functions that are strictly not Oslo method related.

ErlendLima commented 3 years ago

What would be the benefit? I agree with the goal of modularity, but due to how inflexible Python is (e.g a class can only have one definition, each method can only have one definition), and how many moving parts the Oslo method has, decoupling the Matrix and Vector classes from the physics code is impractical. It is possible to write Matrix and Vector to be more OMpy-agnostic, but this would lead to OMpy-code becoming more verbose and difficult to understand.

For example, the plotting of Matrix.plot() only makes sense in the context of Oslo method matrices. Decoupling the plotting from the class (which, in my mind, makes much more Platonic sense. A matrix is a bag of numbers, not a plotting tool) would require the plotting to be rewritten to, say, ompy.plot.plot_raw(matrix), which is less idiomatic Python. In the end, nothing is gained, since matrices are very rarely plotted any other way.

Another example would be the index arrays, such as Matrix.Eg. The names only makes sense when working with Eg-Ex matrices. To make them agnostic of OMpy, they would need a more general name, like Matrix.X or Matrix.E1. In the first versions of OMpy, that was their naming, but this rendered the code that used them difficult to read.

In short, I agree with the intent, but it would introduce more problems than it solves. This is due to the limitations of Python more than OMpy. I have been writing an OMpy clone in Julia, where this problem does not exist, as the implementation of a type and its methods are distinct.

I'm very much open to a better OMpy design, if you have any ideas:)

vetlewi commented 3 years ago

What would be the benefit? I agree with the goal of modularity, but due to how inflexible Python is (e.g a class can only have one definition, each method can only have one definition), and how many moving parts the Oslo method has, decoupling the Matrix and Vector classes from the physics code is impractical. It is possible to write Matrix and Vector to be more OMpy-agnostic, but this would lead to OMpy-code becoming more verbose and difficult to understand.

For example, the plotting of Matrix.plot() only makes sense in the context of Oslo method matrices. Decoupling the plotting from the class (which, in my mind, makes much more Platonic sense. A matrix is a bag of numbers, not a plotting tool) would require the plotting to be rewritten to, say, ompy.plot.plot_raw(matrix), which is less idiomatic Python. In the end, nothing is gained, since matrices are very rarely plotted any other way.

Another example would be the index arrays, such as Matrix.Eg. The names only makes sense when working with Eg-Ex matrices. To make them agnostic of OMpy, they would need a more general name, like Matrix.X or Matrix.E1. In the first versions of OMpy, that was their naming, but this rendered the code that used them difficult to read.

In short, I agree with the intent, but it would introduce more problems than it solves. This is due to the limitations of Python more than OMpy. I have been writing an OMpy clone in Julia, where this problem does not exist, as the implementation of a type and its methods are distinct.

I'm very much open to a better OMpy design, if you have any ideas:)

I might not have made my idea clear enoughπŸ˜… I don't suggest a major change to the design. The point was to make it possible to install OMpy without having to install dependencies such as Multinest. There are lots of useful functionality for manipulating, visualizing and importing/exporting spectra and matrices that doesn't need to rely on external packages.

The way one could achieve this would be to re-organize the package structure to something like:

ompy/
β”œβ”€β”€ core/
β”‚    β”œβ”€β”€ matrix.py
β”‚    β”œβ”€β”€ vector.py
β”‚    └── etc.
β”œβ”€β”€ method/
β”‚    β”œβ”€β”€ unfolder.py
β”‚    β”œβ”€β”€ first_generation.py
β”‚    └── etc.
β”œβ”€β”€ normalizing/     <---- Requires Multinest
β”‚    β”œβ”€β”€ normalize_nld.py
β”‚    β”œβ”€β”€ normalize_gsf.py
β”‚    └── etc.
└── __init__.py

where one can install just the "core" functionality, pip install ompy-core or the entire package pip install ompy.

ErlendLima commented 3 years ago

Ah, I understand! I like the idea:)