Skip to content

Move unnecessary vars in Ansys coputation - #44

Merged
sadda merged 18 commits into
mainfrom
move-unnecessary-vars
Sep 22, 2026
Merged

sadda merged 18 commits into
mainfrom
move-unnecessary-vars

Conversation

@haitao-ZCU

Copy link
Copy Markdown
Collaborator

This builds on PR #31 (still open), so the diff includes some of #31's own files (machine_design/transforms.py, CONTRIBUTING.md, tests/test_transforms.py, tests/test_ansys_transforms.py). The files actually changed by this PR are:

  • motors/synrm_5f_40s.py
  • tests/test_synrm_5f_40s.py
  • tests/test_ansys_regression_synrm_5f_40s.py

Changes:

Testing:

All test files pass, including tests/test_ansys_regression_synrm_5f_40s.py which requires a live Ansys session.

smidl and others added 17 commits September 8, 2026 18:32
… rule

Addresses the additive half of #25.

The same Park/Clarke transform is written out by hand three times in
`motors/synrm_5f_40s.py`'s output variables -- once for flux linkage, once for input
current, once for terminal voltage -- off one pair of cos/sin tables. None of it needs
the field solution; it is algebra on quantities the solve has already produced.

This adds the Python side and the rule, and removes nothing:

  machine_design/transforms.py   the transform once: to_dq / to_phases /
                                 electrical_angle, matching the Ansys convention
                                 exactly, including the sin(-h*(...)) sign that makes
                                 q the negative sine sum
  tests/test_transforms.py       pins that convention offline (no Ansys)
  tests/test_ansys_transforms.py live cross-check on one solve, both subspaces excited
  CONTRIBUTING.md                "What runs in Ansys, what runs in Python"

Phase count is not hard-coded. `to_dq` reads it off the last axis of its input, so the
same function serves the 3-phase anchor and the 5-phase one; `to_phases` takes it
explicitly, having no input array to read it from. The offline tests are parametrised
over (phase count, harmonic), and the m=3/h=3 triplen case -- zero-sequence, so not
invertible -- is pinned as such so nobody "fixes" it.

Verified on a live AEDT 2024.2 solve (bayes, 1 core): the Python transform reproduces
Ansys's own dq output variables to 2.3e-15 (dq1) and 2.5e-16 (dq3) -- floating-point
exact, not within a tolerance.

Two things that verification turned up, both now recorded:

* Ansys returns `theta_el` in DEGREES, not radians -- the `- pi` in its expression is
  evaluated as -180 in an angular context. The Ansys side is correct and internally
  consistent (theta_el is only ever used inside cos/sin as angle-minus-angle), but a
  Python consumer reading the raw number gets silent nonsense. `electrical_angle()`
  therefore takes an explicit `degrees=` flag and always returns radians.
* `Flux_d1..q3` are defined as output variables but never extracted --
  `solution_expressions` requests `Flux_e_*` (excitation flux) instead. So the flux
  transforms are computed by Ansys and discarded.

The cross-check covers flux AND voltage deliberately. Flux stays in Ansys (it needs the
solve), so that half is a permanent guard on the boundary. The voltage half is a
one-shot equivalence proof: the Ansys-side voltage variables are slated to move to
Python, which deletes both sides of the comparison, so it can only be run while they
still exist.

No behaviour change: three new files plus a CONTRIBUTING section.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
separate files applicable to different number of phases, excitation (current vs. voltage), rotation direction, names of phases, ...
@sadda

sadda commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

@haitao-ZCU Just to verify: We are running a test that all the dictionaries returned in extract_results are the same for the current and legacy computation, right? That means not checking only the values, but also that all keys are the same.

One minor change: In self.solution_expressions, please add some for loops to simplify the creation of the list.

@haitao-ZCU

Copy link
Copy Markdown
Collaborator Author

@sadda

  • we only check the shared 23 keys, not all of them. torque_live actually has 31 keys, legacy has 23. The extra 8 (Flux_d1/q1/d3/q3, Vind_d1/q1/d3/q3) exist as formulas in legacy's Ansys output_vars too, they're used there to compute Flux_e_d/q and V_d/q, but legacy's solution_expressions never pulled them out, so there's no reference value on the legacy side to compare against.

  • solution_expressions has been simplified.

@sadda

sadda commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

@haitao-ZCU Thanks. The PR seems to be fine. Would it make sense to create a specific subpackage for transforms? Or since they are so simple, they should be at the root, where they are now?

@haitao-ZCU

Copy link
Copy Markdown
Collaborator Author

@sadda

Good idea.
In my opinion, we should create a folder (e.g. machine_design/generic/ ) to put the generic methods ( "transforms,py" in this PR, and "winding.py" in creating 60 slots (PR #49 ), and so on,), and with these methods, we can parameterize different machines instead of hard-coding.
if we should do this, I prefer to propose a new PR to move transforms,py to machine_design/generic/

@sadda

sadda commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

@haitao-ZCU Ok, I am merging this PR. Please move it in a separate PR.

@sadda
sadda merged commit 9248bf8 into main Sep 22, 2026
6 checks passed
@haitao-ZCU
haitao-ZCU deleted the move-unnecessary-vars branch September 22, 2026 17:24
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.

4 participants