Repository navigation
Fixed Geometry2 - #29
Merged
Merged
Conversation
smidl
added a commit
that referenced
this pull request
Sep 4, 2026
`Geometry2`/`Computation2` are named for being the second machine defined, not for the machine they describe. Nothing in the name says 5-phase or 40-slot, and nothing distinguishes it from the other 5-phase variants people run on branches. One module per anchor, named `<class>_<phases>f_<slots>s`: motors/motor1.py -> motors/synrm_3f_36s.py motors/motor2.py -> motors/synrm_5f_40s.py Each exports plain `Geometry` and `Computation`, so the module name carries the identity and the class names stay uniform. The numbering existed only because both machines shared a namespace; per-anchor modules remove the need for it. from motors.synrm_5f_40s import Computation, Geometry The name carries only what the anchor fixes for good -- class, phases, slots. Turns are deliberately left out: `Nc` is routinely swept, so a name containing it goes stale. Same reasoning for currents and bus voltage. The old paths remain as deprecation shims re-exporting `Geometry2`/`Computation2`, so branches that predate this keep importing and can migrate on their own schedule. Also restores the anchor definition to CONTRIBUTING (it survives on the `contributing` branch but was dropped in #23, leaving "anchor" used three times and never defined, and the sentence "add an **motors**" mid-substitution), and fixes that sentence. No behaviour change. Both anchor modules were verified content-identical to the `motor1.py`/`motor2.py` they replace, modulo the class renames and a one-line docstring -- including the fixes from #28 and #29, which landed in `motor2.py` while this branch was open and had to be carried across by hand. That is worth noting: a rename PR gets *no merge conflict* when a fix lands in the file it moves, because the new path is an unrelated add. Suite is 26 passed / 1 deselected, identical to main; Ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
smidl
added a commit
that referenced
this pull request
Sep 6, 2026
`Geometry2`/`Computation2` are named for being the second machine defined, not for the machine they describe. Nothing in the name says 5-phase or 40-slot, and nothing distinguishes it from the other 5-phase variants people run on branches. One module per anchor, named `<class>_<phases>f_<slots>s`: motors/motor1.py -> motors/synrm_3f_36s.py motors/motor2.py -> motors/synrm_5f_40s.py Each exports plain `Geometry` and `Computation`, so the module name carries the identity and the class names stay uniform. The numbering existed only because both machines shared a namespace; per-anchor modules remove the need for it. from motors.synrm_5f_40s import Computation, Geometry The name carries only what the anchor fixes for good -- class, phases, slots. Turns are deliberately left out: `Nc` is routinely swept, so a name containing it goes stale. Same reasoning for currents and bus voltage. Also restores the anchor definition to CONTRIBUTING (it survives on the `contributing` branch but was dropped in #23, leaving "anchor" used three times and never defined, and the sentence "add an **motors**" mid-substitution), and fixes that sentence. No behaviour change: the two anchor modules were diffed against the `motor1.py` / `motor2.py` they replace, with the class renames normalised away, and are identical apart from a one-line docstring -- including the fixes from #28 and #29, which landed in `motor2.py` while this branch was open and had to be carried across by hand. Callers must update their import path; there is no compatibility shim, so a stale import fails loudly rather than silently resolving to an aliased class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removed the function
set_derived_paramswhich overwrote the original one. This removed the bug when therotor_r_minattribute was not defined.