Change module structure - more classes

[GiovanniKl]

My idea is to have more classes with more specific functionalities, e.g. base class DispersionCharacteristic, and derived classes for SAFs (e.g. DispersionSAF), parametric pumping (e.g. DispersionParaPump), etc.

This should help with:

• better orientation within the module (less cramped code)
• easier modifications targeting specific functionalities

On the other hand, any functionalities that some of these classes might have in common (and which are not part of the parent class) would be compromised. However, I am not aware of any such relationship. Correct me if I'm wrong, please.

I know something like this has already been discussed, but I'm not sure if this extenstion is still something we all agree on, so please give your ideas.

---

main conclusions:

Remember, these changes should be done to the new branch named reclass, not the master branch. Tick the corresponding box when you finish some of these points.

• divide DispersionCharacteristic into three standalone (first-order) classes:
  • [ ] SingleLayer - Slavin-Kalinikos (zeroth and second perturbation), partially-pinned kappa, parametric pumping
  • [ ] SingleLayerNumeric - Tacchi
  • [ ] DoubleLayerNumeric - SAFs
• [ ] add a new class KittelFMR with calculation of the Kittel formula, PSSW modes (Kittel-Herrings formula), B_eff, and equilibrium angle (inspire from .ipynb files in BiYIG repository)
• [ ] remove the Rezende approach
• [ ] change structure of module files for improved readability
  • SpinWaveToolkit - base folder (copy this folder to install the module instead of copying just the original SWT python file)
  • __init__.py - file where all useful classes, functions and constants are imported
  • core - folder with all classes as individual scripts (e.g. for Material class named _class_Material.py)
  • helpers.py - functions defined within the module (can be imported into class files easily if needed, [this file should not have any imports from other parts of this module I think]), contains also constants (except for pre-defined Material instances, which should be defined in their respective class file and only imported if needed)
• [ ] update the README.md and CONTRIBUTING.md (best after accomplishing all above)

[OndrejW]

I agree to make specific class for SAFs (maybe will be better to rename it to just bilayer, as it can calculate also these - with arbitrary separation and only dipolar coupling). I think the parametric pumping would be better inside the original class (maybe we should rename to DispersionCharSingleLayer?).

IMO the question is if we should separate Tacchi from original class?

[GiovanniKl]

It seems to me that the Tacchi model is quite specific. It might be worth to move it aside, so that you don't carry all that load with you every time you want to calculate everything else (derived classes from base class would inherit it also).

For the names, I suggest the following:

• DispersionCharLayer - base class, includes the Kalinikos-Slavin and their second perturbation, partially pinned kappa, parametric pumping
• DispersionTacchi - child of base, the Tacchi model
• DIspersionBilayer - child of base, SAF-concerned stuff

AFAIK, this should be all that we have right now.

Also, we might think of making the module a bit more spread in multiple files. I don't like how long the file starts to be (and will be even more with the dilution into aforementioned classes). Even pylint yells at me that the module is longer than recommended 1000 lines 😆 (ofc this limit can be increased in the setup, but I'd see it as a better practise to have more smaller files). This would include making a folder SpinWaveToolkit with folders and files inside, mainly containing the __init__.py file. I'd recommend getting some inspiration in TetraX or magpylib.

[OndrejW]

This is perfect idea!! I suggest following names for classes: SingleLayer, SingleLayerNumeric, a DoubleLayer (Maybe we can consider DoubleLayerNumeric, so that it will be little bit more clear...)

I like the idea with child classes! However, I am not sure if there are any inherited methods which can be of any use for SingleLayerNumeric and DoubleLayer. And inherited methods for group velocity and lifetime etc. from SingleLayer can be misleading.

Concerning the splitting of the code, I think that this will enhance the readability a lot. Do you have any idea how to split the file?

[OndrejW]

Maybe can @michalurb give his opinion before we start to change the structure?

[GiovanniKl]

I suggest following names for classes: SingleLayer, SingleLayerNumeric, a DoubleLayer (Maybe we can consider DoubleLayerNumeric, so that it will be little bit more clear...)

I really like those names. Yeah, the DoubleLayerNumeric sounds maybe better,

I like the idea with child classes! However, I am not sure if there are any inherited methods which can be of any use for SingleLayerNumeric and DoubleLayer. And inherited methods for group velocity and lifetime etc. from SingleLayer can be misleading.

It's true that some inherited methods could cause trouble. In that case I agree with making these three classes as standalone ones.

Concerning the splitting of the code, I think that this will enhance the readability a lot. Do you have any idea how to split the file?

I think I know how to do it. We would just have to agree on the structure. I'm not sure in what directions we want to expand in the future.

[michalurb]

I vote for SingleLayer, SingleLayerNumeric, DoubleLayerNumeric. What we will do with Rezende approach? Should we delete it, because it does not work?

And I also propose to add KittelFMR class see my answer here: https://github.com/CEITECmagnonics/SpinWaveToolkit/issues/13#issuecomment-2167871445

[GiovanniKl]

Great. I agree. See the updated first comment, and if you agree with the conclusions so far, put a like react to this/new comment.

Concerning the Rezende approach, I cannot say much, @OndrejW should comment on this.

[GiovanniKl]

I have come up with a structure for our files (being inspired by other modules):

• SpinWaveToolkit - base folder (copy this folder to install the module instead of copying just the original SWT python file)
  • __init__.py - file where all useful classes, functions and constants are imported
  • core - folder with all classes as individual scripts (e.g. for Material class named _class_Material.py)
  • helpers.py - functions defined within the module (can be imported into class files easily if needed, [this file should not have any imports from other parts of this module I think])

I thought about where to define our constants (like MU0), and I think placing them in the helpers.py file makes the most sense, and the pre-defined materials could be in the script with the Material class.

What do you think?

[OndrejW]

Great. I agree. See the updated first comment, and if you agree with the conclusions so far, put a like react to this/new comment.

Concerning the Rezende approach, I cannot say much, @OndrejW should comment on this.

Yes, we can remove this model entirely from SWT

[OndrejW]

I have come up with a structure for our files (being inspired by other modules):

* `SpinWaveToolkit` - base folder (copy this folder to install the module instead of copying just the original SWT python file)

  * `__init__.py` - file where all useful classes, functions and constants are imported
  * `core` - folder with all classes as individual scripts (e.g. for `Material` class named `_class_Material.py`)
  * `helpers.py` - functions defined within the module (can be imported into class files easily if needed, [this file should not have any imports from other parts of this module I think])

I thought about where to define our constants (like MU0), and I think placing them in the helpers.py file makes the most sense, and the pre-defined materials could be in the script with the Material class.

What do you think?

To me, this seems as very good approach, lets go this way!