Loss models

[astonysheen]

trafficstars

This pull request improves the loss module of PYLEECAN. The architecture has been changed to make it easier to define new loss models and to choose which ones to use in the simulations. A script for the computation of efficiency maps has been added. All the equations, the references, and the instructions to use the new loss module are detailed in the notebook tuto_loss.ipynb.

[SebGue]

Hello,

I haven't reviewed this PR in its actual context but found some kind of general issue, i.e. 'LossModelMagnet.comp_loss' refer to specific geometries (HoleMx, ...) to get magnet width and height. Instead there should be some common Lam or HoleMx methods to get this quantities. Alternatively magnet dimensions should be extracted from FEA mesh. Otherwise every new geometry requires to append comp_loss.

Best regards, Sebastian

[BonneelP]

Hello,

Thank you for the remark, I will keep that in mind while reviewing the PR. There is also a rework on add_model and remove_model that may require your feedback.

Sadly I don't have the time currently to review the PR. First there was ICEM and then last week we moved to a new office. Both event were very time consuming before, during and after. I hope to be able to review/close some PR next week. Sorry for the delay. Best regards, Pierre

[BonneelP]

Hello,

I finally had the time to check this PR, I did few code rework and cleaning and all the tests are now passing. I think that it could be interesting to merge this PR as it is currently since it add new models and a loss tutorial. There are some parts that need improvements to make theses models more maintainable but it could take time to implement the solution (and it shouldn't change the model results).

Here is what I have noted:

• As said by Sebastian, we need to rework LossModelMagnet.comp_loss. Either by adding methods get_Wmag/get_Hmag or by reworking the formula to use surfaces rather than height/width. In the current version of the code, in fact only Wmag is needed to compute segmentation coefficient. Maybe we should also consider that a machine can have several magnets of different shape.
• We now have simu.elec.Tsta and simu.loss.Tsta for the same purpose which will lead to errors (I don't think that we need to have the possibility to set a different temperature for theses models on a same OP). I would suggest to add all the temperature (Tsta, Trot, Tmag) to OP, which would one day enable to define OP_matrix with temperature columns.
• There are duplicated code to compute the skin effect. LossModelWinding.comp_coeff and LossModelJoule.comp_coeff are identical and almost matches Conductor.comp_skin_effect_resistance. I think that we should be able to merge comp_coeff formula into comp_skin_effect_resistance to remove both comp_coeff (or add a new method to Conductor).
• Now the loss models are defined in Simu.loss.model_dict, out.loss.store will call the comp_loss of each model in the dict and generate a OutLossModel for each with a property "name" (which is the model_dict key) used for post-processing naming and the OutLossModel are store in out.loss.loss_list. I'm wondering if for consistency we should replace loss_list by loss_out_dict (and keep name property since its convenient for the plot).

@SebGue what do you think about these modifications proposal ? If you agree I can merge this PR and open an issue with these improvements as the next step for losses models.

Best regards, Pierre

[SebGue]

Hello @BonneelP

I will check the PR the next days.

Best Regards, Sebastian

[SebGue]

Hello @BonneelP

finally I got some time to review this PR. The PR will be a great addtion to pyleecan. Moving to a dict for models simplifies loss simulation setup a lot. Further there are some nice features like adding solutions to an overall solution. What I want to note is:

• I think LossFEMM should be renamed to LossFEA to allow other magnetic solvers that generate a MeshSolution.
• There could be a property or method to clear Magnetic.MeshSolution after loss calculation since some users may not be interessted in loss and flux distribution and want to save some memory.
• I agree with your out_dict proposal. Therefore we could also adapt XOutput's dict-like feature.
• The actual loss computation is in OutLoss which is not favorable in my opinion.
• Loss assumes that there is a MeshSolution, i.e. in OutLoss.store there is group = meshsol.group. That will cause an error for mag.meshsolution == None. There should be robustness for missing MeshSolution even if is_get_meshsolution == True. Further there are loss models that don't rely on MeshSolution, e.g. DC winding losses, windage losses, ...
• Could you find another name for LossModelWindage, since there are some other windage loss calculation methods.
• Loss (i.e. OutLoss) assumes "triangles" which isn't favorable too. Maybe you can have a look at my other issue #567 that addresses this point.
• In the comp_coeff methods, there are function definitions. I don't know if this is good practice and if they could be excluded?

Hope there are some helpful hint :-)

Best regards, Sebastian

[SebGue]

... I haven't checked the single models for now. Is there a test to check all possible periodicities in space and time?

[BonneelP]

Hello Sebastian,

Thank you for taking the time to review this PR. Regarding your returns:

• I think LossFEMM should be renamed to LossFEA to allow other magnetic solvers that generate a MeshSolution.

This class is linked to a particular FEMM validation case and for now all losses computation are FEA based. But for future evolution I agree that we can rename it. Will be done before merge

• There could be a property or method to clear Magnetic.MeshSolution after loss calculation since some users may not be interessted in loss and flux distribution and want to save some memory.

There is nothing to clean afterwards, but one can select if the mesh needs to be store with Loss.is_get_meshsolution and MagFEMM.is_get_meshsolution. So one could keep the mag mesh for loss computation (MagFEMM.is_get_meshsolution=True) and discard losses mesh (Loss.is_get_meshsolution=False).

• I agree with your out_dict proposal. Therefore we could also adapt XOutput's dict-like feature.

Great idea to reuse the "XOutput's dict-like feature", I will add that before the merge.

• The actual loss computation is in OutLoss which is not favorable in my opinion.

I fully agree, it was confusing while reading the code. I will try to find some time to do better (if I don't have the time, I will add it to the issue "to go further with losses")

• Loss assumes that there is a MeshSolution, i.e. in OutLoss.store there is group = meshsol.group. That will cause an error for mag.meshsolution == None. There should be robustness for missing MeshSolution even if is_get_meshsolution == True. Further there are loss models that don't rely on MeshSolution, e.g. DC winding losses, windage losses, ...

While reworking the "store-compute", we can investigate removing this assumption to ease future work. I think that we can have OutLossModelFEA as a daughter of OutLossModel that would then be used for scalar output or is_get_meshsolution=False.

• Could you find another name for LossModelWindage, since there are some other windage loss calculation methods.

I don't find any name inspiration in the docstrings/comment. Maybe simply LossModelWindage1 ?

• Loss (i.e. OutLoss) assumes "triangles" which isn't favorable too. Maybe you can have a look at my other issue Solution.type_cell #567 that addresses this point.

I will take a look once this PR done

• In the comp_coeff methods, there are function definitions. I don't know if this is good practice and if they could be excluded?

These methods can be improved indeed, I will had than to the "to go further with losses" issue.

Thanks again, Best regards, Pierre

[SebGue]

Hello Pierre,

a name proposal from my side is LossModelwindagePyrhonen. But you can also keep the actual name and all other windage models will have an addtional identifier.

Maybe we should move loss models to its own sub folder for more clarity.

In the tutorial there is section 'Skin effect' with some inconsistency regarding formula symbols. Further there could be something like "kappa_eff = kappa_conductor / k_skin" for completeness.

Best regards, Sebastian

[BonneelP]

Hello Sebastian,

Finally coming back to this PR. I will try to summarize the discussion in a TODO list (fell free to complete it):

• [x] Rename LossFEMM => LossFEA
• [x] - Move Loss classes to new folder outside of Simulation
• [x] - Rework loss_list to loss_dict in OutLoss
• [x] - Rework OutLoss.store that compute the losses
• [x] Add back self.scalar_value (to be used for losses without mesh)
• [ ] rework LossModelMagnet.comp_loss (Wmag/Hmag)
• [ ] Move all Temperature in OP
• [ ] Remove duplicated code to compute the skin effect
• [x] XOutput's dict-like feature.
• [ ] clear meshsolution after run
• [x] Rename LossModelWindage to LossModelwindagePyrhonen
• [x] pyleecan/Methods/Output/LUTdq/interp_Ploss_dqh.py
• [ ] Update tutorial
• [ ] Release + exe + website

Best regards, Pierre