You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
verify winding_layout_JL.py and incorporate it to main #66
changed the title [-]verify validate_winding_layout_JL.py and incorporate it to main[/-][+]verify winding_layout_JL.py and incorporate it to main[/+]on Oct 13, 2026
Fractional slot winding
I also tested a machine with 12 slots and 10 poles.
winding.py raises a ValueError because q is not an integer. winding_layout_JL.py works and gives a winding factor of 0.966. This means that winding_layout_JL.py can support fractional slot windings, while winding.py only support limited integer slot windings.
Something I couldn't understand and confused:
There's a difference between winding_layout_JL.py and our current FEM model.
For the 3f_36s machine:
winding_layout_JL.py give p2_min_fem=2 (lines 139-150), this means we should use two poles with periodic boundaries.
While our current FEM model uses one pole with anti periodic boundaries. (line 180 in mechine_design/designs/geometry.py and line 153 in motors/synrm_3f_36s.py).
And I also checked the phase and polarity of all 36 slots, and the winding pattern does follow the expected symmetry for one pole with anti periodic boundaries.
I couldn't understand why we should set p2_min_fem=2 in this case. @saxik13 or does the formula consider something else?
Nest steps:
I have only tested the winding calculations, I haven't test winding_layout_JL.py with an actual Ansys simulation, so my plan is:
add winding_layout_JL.py to the existing model on a separate branch
run the Ansys tests for the 3f and 5f machines and compare the results. @sadda One thing I'm not sure is that whether we should replace the old winding.py completely with winding_layout_JL.py, or keep both (winding_layout_JL.py only for fractional slots)? I prefer to replace completely because winding_layout_JL.py is more general and covers integer and fractional cases, but I need to do more tests, including Ansys tests.
Thank you @haitao-ZCU for the comparison. I have checked the decision tree regarding the FEM minimum number of poles. If the number of layers (coils in one slot) equals one, numerator of the number of slots per pole and phase decide (odd or even). That is correct. Windings with full pitch (coil_pitch = pole_pitch_slot ~ belt_offset = 0) are generally an exception. You can check the phase sequence for 36 slots. If coil_pitch=7,5,11, etc., you need 2 poles to get the symmetry. Only for coil_pitch = 9 is symmetry ensured with one pole in FEM.
We can just change line 150 to:
p2_min_fem = slots_ppp_denom if coil_pitch == pole_pitch_slots else 2*slots_ppp_denom
@haitao-ZCU Do I understand correctly that you run the comparison only for a selected number of parameters? If so, please make the comparison on a grid of phase = 3, 5, 7, 9, whatever, belt-offsets = 0..n_phases and other parameters in a similar way. I guess that it is a comparison of simple functions, and it should be feasible even for a large run. Then please send the script which you used for comparison. No need to include it in the tests or formally in the repo.
@haitao-ZCU We want to replace it. First, we want to make sure that it gives the same results (which you have already partially done).
@haitao-ZCU If I understand the code correctly, it only checks the transformation between belt_offset and coil_pitch. Would it be possible to make a much broader comparison of whether the old and the new code give the same windings?
60 combinations tested: (phases=3/5/7/9/11, pole_pairs=1-4, 3 slot densities each): old and new give identical slot/phase/polarity assignments every time.
(phase_a_axis-180)/ pole_pairs equals computed InitPos (computed from compute_initpos.py) every time.
Fractional slots: old raises error, new gives a valid result (tested 12-slot/10-pole).
Even phase counts: old winding.py can't support even phase, while the new one supports.
@haitao-ZCU Do I understand correctly that we are ready to replace the old windings with winding_layout_JL.py? If so, please do it. No need to include it as proper tests, the existing tests for the Ansys regression should be sufficient.
Activity
[-]verify validate_winding_layout_JL.py and incorporate it to main[/-][+]verify winding_layout_JL.py and incorporate it to main[/+]haitao-ZCU commentedon Oct 13, 2026
Progress update:
I have made some tests and comparisons between winding.py and winding_layout_JL.py
Results are as follows:
If pole_pitch_slots = slots / (2 * pole_pairs) is:
Fractional slot winding
I also tested a machine with 12 slots and 10 poles.
winding.py raises a ValueError because q is not an integer. winding_layout_JL.py works and gives a winding factor of 0.966. This means that winding_layout_JL.py can support fractional slot windings, while winding.py only support limited integer slot windings.
Something I couldn't understand and confused:
There's a difference between winding_layout_JL.py and our current FEM model.
For the 3f_36s machine:
I couldn't understand why we should set p2_min_fem=2 in this case. @saxik13 or does the formula consider something else?
I have only tested the winding calculations, I haven't test winding_layout_JL.py with an actual Ansys simulation, so my plan is:
@sadda One thing I'm not sure is that whether we should replace the old winding.py completely with winding_layout_JL.py, or keep both (winding_layout_JL.py only for fractional slots)? I prefer to replace completely because winding_layout_JL.py is more general and covers integer and fractional cases, but I need to do more tests, including Ansys tests.
saxik13 commentedon Oct 14, 2026
Thank you @haitao-ZCU for the comparison. I have checked the decision tree regarding the FEM minimum number of poles. If the number of layers (coils in one slot) equals one, numerator of the number of slots per pole and phase decide (odd or even). That is correct. Windings with full pitch (coil_pitch = pole_pitch_slot ~ belt_offset = 0) are generally an exception. You can check the phase sequence for 36 slots. If coil_pitch=7,5,11, etc., you need 2 poles to get the symmetry. Only for coil_pitch = 9 is symmetry ensured with one pole in FEM.
We can just change line 150 to:
p2_min_fem = slots_ppp_denom if coil_pitch == pole_pitch_slots else 2*slots_ppp_denom
sadda commentedon Oct 14, 2026
@haitao-ZCU Do I understand correctly that you run the comparison only for a selected number of parameters? If so, please make the comparison on a grid of phase = 3, 5, 7, 9, whatever, belt-offsets = 0..n_phases and other parameters in a similar way. I guess that it is a comparison of simple functions, and it should be feasible even for a large run. Then please send the script which you used for comparison. No need to include it in the tests or formally in the repo.
@haitao-ZCU We want to replace it. First, we want to make sure that it gives the same results (which you have already partially done).
haitao-ZCU commentedon Oct 14, 2026
@saxik13 Thanks for checking and the fix suggestion. It now works correctly.
@sadda Yes, the earlier comparison was only a selected number of parameters. I have modified it into a grid sweep:
phases = 3/5/7/9/11, pole_pairs = 1-4, q (slots per pole per phase) = 1-3 (60 combinations total).
All 60 combinations passed and 0 mismatches. Scripts link is:
https://github.com/AI-RICE/MachineDesign/blob/investigate-winding-layout/scripts/match_winding_params_grid.py
So, I'll start a separate branch to incorporate winding_layout_JL.py and do Ansys tests.
sadda commentedon Oct 14, 2026
@haitao-ZCU If I understand the code correctly, it only checks the transformation between
belt_offsetandcoil_pitch. Would it be possible to make a much broader comparison of whether the old and the new code give the same windings?haitao-ZCU commentedon Oct 14, 2026
@sadda
It compares the phase and polarity one by one through "if old == new:"(line 36). I also made a comparison between winding.py (old, or current project files) and winding_layout_JL.py (new) and added some validations. The script link is:
https://github.com/AI-RICE/MachineDesign/blob/investigate-winding-layout/scripts/match_winding_params_grid.py
Results
haitao-ZCU commentedon Oct 15, 2026
Progress update:
Ansys tests passed.
I use assign_phase_windings_new (line 94) in computation.py to connect winding_layout_JL.py. This method uses compute_winding_layout and assign_slot_currents_to_aedt to assign the windings in Ansys.
In order to validate the results, I update the existing regression tests to compare three versions: original code (legacy), current code (live), and the new winding code (new). Test links are:
3f_36s:
https://github.com/AI-RICE/MachineDesign/blob/66-verify-winding_layout_jlpy-and-incorporate-it-to-main/tests/test_ansys_new_winding_regression_synrm_3f_36s.py
5f_40s:
https://github.com/AI-RICE/MachineDesign/blob/66-verify-winding_layout_jlpy-and-incorporate-it-to-main/tests/test_ansys_new_winding_regression_synrm_5f_40s.py
Test results are as follows:
Both machines are tested with two different random rotor designs (seeds=42, 43).
Compared quantities:
All results matched within the tolerance (relative tolerance=1e-3, absolute tolerance=2e-5 for 5f test).
Summary
Using winding_layout_JL.py now gives the same Ansys simulation results as the current project code for both tested machines.
sadda commentedon Oct 16, 2026
@haitao-ZCU Do I understand correctly that we are ready to replace the old windings with
winding_layout_JL.py? If so, please do it. No need to include it as proper tests, the existing tests for the Ansys regression should be sufficient.