Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major rework of input generation and electrical models #464

Merged
merged 209 commits into from
Nov 2, 2021

Conversation

EmileDvs
Copy link
Collaborator

@EmileDvs EmileDvs commented Oct 27, 2021

This pull request is in progress to solve main issue #452

  • gather conventions for rotating direction in an .init file (rot_dir_ref=current_dir_ref=phase_dir_ref=-1)
  • finish validation case for LUTslip
  • validate all tests

Jean Le Besnerais and others added 30 commits April 26, 2021 14:47
…can (Random fswi and Symetrical random fswi)
# Conflicts:
#	Tests/Methods/Import/test_ImportGenPWM.py
#	pyleecan/Classes/Electrical.py
#	pyleecan/Classes/ImportGenPWM.py
…tVoltage

[WiP] Add test_EEC_ELUT_SCIM_001.py to test EEC_SCIM using ELUT import
@EmileDvs
Copy link
Collaborator Author

EmileDvs commented Oct 28, 2021

Hello Emile,

that is a very large PR with lot of changes. Is there a unit test to see what the actual intended workflow is?

Best regards, Sebastian

Hello @SebGue,

This a very large PR indeed, it changes a lot of objects in pyleecan but it is clearly for the best :) After this PR we will probably do a new pyleecan major release.

Here is a list of the tests for the new features/reorganization (the checked one are finished):

  • rework of Inputs objects (InputCurrent + OPdq) and new conventions: Tests\Methods\Simulation\test_mmf_dir.py
  • rework of periodicity calculation: Tests\Methods\Machine\test_comp_periodicity.py
  • dqh transformation: Tests\Functions\test_dqh_transformation.py
  • Generation of PWM voltage (InputVoltage): Tests\Methods\Simulation\test_InVoltage_PWM.py
  • EEC_PMSM + look up table (LUTdq) for MTPA: Tests\Validation\Electrical\test_EEC_ELUT_PMSM.py
  • EEC_PMSM + look up table (LUTdq) for PWM currents calculation: Tests\Validation\Electrical\test_EEC_ELUT_PMSM.py
  • EEC_SCIM + look up table (LUTslip) for rotor bar currents + average torque calculation: Tests\Validation\Electrical\test_EEC_ELUT_SCIM_001.py

For test_mmf_dir.py, I still have to gather conventions for rotating direction in an .init file (rot_dir_ref=current_dir_ref=phase_dir_ref=-1), probably the .init file in Input method folder.

For test_EEC_ELUT_SCIM_001.py, there is still some clean up to do.

I detailed the different developments in the different issues gathered in global issue #452 .

Best regards,
Emile

@SebGue
Copy link
Collaborator

SebGue commented Oct 28, 2021

Hello @EmileDvs

thanks for your answer. I will have a look at all your questions/code changes. I will try to give some feedback uptill the beginning of next week.

BR S

# Conflicts:
#	Tests/Validation/Electrical/test_EEC_PMSM.py
#	Tests/Validation/Magnetics/test_FEMM_periodicity.py
#	pyleecan/Classes/MagElmer.py
#	pyleecan/Classes/MagFEMM.py
#	pyleecan/Classes/Magnetics.py
#	pyleecan/Classes/import_all.py
#	pyleecan/Functions/load_switch.py
#	pyleecan/Generator/ClassesRef/Simulation/Magnetics.csv
#	requirements.txt
[NF] Add method in dqh_transformation.py to calculate phase rotating direction from an n-phase signal
@SebGue
Copy link
Collaborator

SebGue commented Nov 1, 2021

Hello @EmileDvs

I have reviewed the code, so here we go :-)
(I skipped all the changes in Input, transformations, ... and concentrated on the EEC 'core' for now.)

At first some technically notes.
I think in 'Electrical.run' method all called EEC methods should be moved into e.g. EEC.solve_EEC. So EEC classes have to take care of the correct parameters on its own. Otherwise the abstract EEC class should have all the required props and methods, so the EEC structure is enforced for inherited classes.
Further the init of an empty (None) Electrical.eec self.eec = EEC_PMSM() should not work since self.eec.fluxlink will be None (see EEC_PMSM.comp_parameters).
In EEC you are calculating some skin effect. Could you tell something about it. (I guess it pure conductor effect without slot proximity effect.) Also I saw a factor on the fluxlinkage, I haven't heard of for now.

Now all the following points are maybe because I use my EEC a little different (in a class I called 'PerformanceOutput') and I don't know the very intention of your Electrical class usage.
I'm most of the time have a requested torque and speed (with bounded max. voltage and current). So I calculate current and voltage with my EEC based on a torque/speed OP. This means the control strategy is already part of my PerformanceOutput class. (I will anwser on the MTPA issue right after that, so it maybe clearer.) Also there are no losses (beside winding losses) included in the EEC that are essential for my use. (That are e.g. core losses, mechanical losses and addtional losses.)
So the question is, could that be part of the actual Electrical/EEC implementation without messing up the code.

BR S

@EmileDvs
Copy link
Collaborator Author

EmileDvs commented Nov 2, 2021

Hello @SebGue,

Thanks for your quick feedback :)

I think in 'Electrical.run' method all called EEC methods should be moved into e.g. EEC.solve_EEC. So EEC classes have to take care of the correct parameters on its own. Otherwise the abstract EEC class should have all the required props and methods, so the EEC structure is enforced for inherited classes. Further the init of an empty (None) Electrical.eec self.eec = EEC_PMSM() should not work since self.eec.fluxlink will be None (see EEC_PMSM.comp_parameters).

I think we can indeed improve how EEC object is handled in Electrical.run() . Putting the methods in EEC.solve_EEC can solve the issue but then we would have to look for the ELUT in self.parent.ELUT_enforced which is not really generic.

You point out the fact that we have to make EEC.comp_parameters() compatible with LUT, in particular when we call self.fluxlink.comp_fluxlinkage() and self.indmag.comp_inductance(). To do so, we can create a FluxLinkLUT and IndMagLUT to mirror FluxLinkFEMM and IngMagFEMM, as children of FluxLink and IndMag abstract classes, with same methods so it can be transparent.

To go a step further, we can merge FluxLink and IndMag objects into a single object, e.g. "ElecParam", with two methods comp_fluxlinkage() and comp_inductance() which are in fact very close (calling the model, calculating stator winding flux with/without currents, returns flux or inductance by dividing flux by current).

Finally for the present case, we would have "ElecParamFEMM" and "ElecParamLUT", the first one calling MagFEMM in comp_fluxlinkage() and comp_inductance() and the second one taking a LUT as input and calling LUT methods in comp_fluxlinkage() and comp_inductance().

In EEC you are calculating some skin effect. Could you tell something about it. (I guess it pure conductor effect without slot proximity effect.) Also I saw a factor on the fluxlinkage, I haven't heard of for now.

I think you are right about the skin effect model neglecting slot proximity effect but I'm not sure. The skin_effect model may be not validated yet, it's mostly important for SCIM to get skin effect in rotor bars. I don't know the reference for accouting skin effect in flux linkage, in any case it can also be set to 1 for now.

Now all the following points are maybe because I use my EEC a little different (in a class I called 'PerformanceOutput') and I don't know the very intention of your Electrical class usage. I'm most of the time have a requested torque and speed (with bounded max. voltage and current). So I calculate current and voltage with my EEC based on a torque/speed OP. This means the control strategy is already part of my PerformanceOutput class. (I will anwser on the MTPA issue right after that, so it maybe clearer.) Also there are no losses (beside winding losses) included in the EEC that are essential for my use. (That are e.g. core losses, mechanical losses and addtional losses.) So the question is, could that be part of the actual Electrical/EEC implementation without messing up the code.

We use Electrical class to mainly get current from voltage, and from this we can calculate magnetic flux and forces. However I definitely think the "PerformanceOutput" class should part of the Electrical model and extend the loss calculation model to other losses.

Is it ok for you if we merge soon the branch in pyleecan/master and then start a new common branch on EOMYS-Public to merge LUT formats and implement 'PerformanceOutput' ? That way we could start to work on a new pull request since this one is already quite huge.

Best regards,
Emile

@SebGue
Copy link
Collaborator

SebGue commented Nov 2, 2021

Hello @EmileDvs

I will answer your last question (and your question in #466) first.
If it is okay that there may be some e.g. structural changes to Electrical and EEC with my future merges, you can merge this PR to master. (Also if you need some of the actual changes for your Manatee.)
Otherwise you could create a feature branche e.g. Eomys\EEC_rework and PR and merge your work there.
So I can also PR to this branch. But maybe you could create such branch anyway for that reason. (Although I got some user rights on Eomys, I think you will be faster doing that since I haven't setup a respective remote repo.)

BR Sebastian

@SebGue
Copy link
Collaborator

SebGue commented Nov 2, 2021

... self.parent.ELUT_enforced (which) is not really generic ...

ELUT could be an optional input parameter of solve_eec. With my implementation LUT is even part of EEC.

You point out the fact that we have to make EEC.comp_parameters() compatible with LUT ...

Oh, my point was not the parameters but the init of self.eec in case self.eec is None which won't work. In that case self.fluxlink is init to None and ECC.comp_parameters() has a check that will raise an exception.
Anyway I haven't reviewed Fluxlink and IndMag but I think there is a chance that this could even be a single object with LUT.

... skin effect ...

I saw some references in the code
"cf. SKIN EFFECT, PROXIMITY EFFECT AND THE RESISTANCE OF CIRCULAR AND RECTANGULAR CONDUCTORS, page 6",
"cf. SKIN EFFECT, PROXIMITY EFFECT AND THE RESISTANCE OF CIRCULAR AND RECTANGULAR CONDUCTORS, page" and
" # p257 Pyrhonen"
Do you know these? Are they freely avialable?

@EmileDvs
Copy link
Collaborator Author

EmileDvs commented Nov 2, 2021

Hello @EmileDvs

I will answer your last question (and your question in #466) first. If it is okay that there may be some e.g. structural changes to Electrical and EEC with my future merges, you can merge this PR to master. (Also if you need some of the actual changes for your Manatee.) Otherwise you could create a feature branche e.g. Eomys\EEC_rework and PR and merge your work there. So I can also PR to this branch. But maybe you could create such branch anyway for that reason. (Although I got some user rights on Eomys, I think you will be faster doing that since I haven't setup a respective remote repo.)

BR Sebastian

Alright, in that case I am going to merge EEC and I have opened a branch "EEC_rework" on EOMYS-Public: https://github.com/EOMYS-Public/pyleecan/tree/EEC_rework

In this PR the part related to LUT of SCIM is still in progress and will be adressed in the EEC_rework.

@EmileDvs
Copy link
Collaborator Author

EmileDvs commented Nov 2, 2021

ELUT could be an optional input parameter of solve_eec. With my implementation LUT is even part of EEC.

Oh, my point was not the parameters but the init of self.eec in case self.eec is None which won't work. In that case self.fluxlink is init to None and ECC.comp_parameters() has a check that will raise an exception. Anyway I haven't reviewed Fluxlink and IndMag but I think there is a chance that this could even be a single object with LUT.

You're probably right, the main interest of FluxLinkFEMM, IndMagFEMM classes and eec.fluxlink/eec.indmag attributes is to calculate those quantities within the workflow but we'll probably not use this workflow eventually.

The most common workflow will be:

  1. build the LUT from simulation with VarLoadCurrent for a grid of Id/Iq
  2. interpolate EEC parameters within the LUT for a given Id/Iq

If solve_EEC() uses LUT by default, there is no need to keep LUT in Electrical rather than in EEC object, it makes sense. We can keep this in mind for EEC_rework.

I saw some references in the code "cf. SKIN EFFECT, PROXIMITY EFFECT AND THE RESISTANCE OF CIRCULAR AND RECTANGULAR CONDUCTORS, page 6", "cf. SKIN EFFECT, PROXIMITY EFFECT AND THE RESISTANCE OF CIRCULAR AND RECTANGULAR CONDUCTORS, page" and " # p257 Pyrhonen" Do you know these? Are they freely avialable?

Ah yes it probably comes from Pyrhonen book which is not freely available but we have this book and I can provide you with scans.

@Eomys Eomys merged commit b6104da into Eomys:master Nov 2, 2021
@SebGue
Copy link
Collaborator

SebGue commented Nov 5, 2021

@EmileDvs If you could provide the scans that would be great. But you don't have to hurry on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants