Skip to content

Fix hard-coded number of phases #57

Closed

Description

@sadda

The number of phases and the belt_offset describe the winding, so they should be defined in Geometry as n_phases and belt_offset. Currently they are not:

  • they are set as self.phases / self.belt_offset in Computation.set_oper_params,
  • they are additionally hard-coded in phase_groups(Q, p, 3, belt_offset=0) / phase_groups(Q, p, 5, belt_offset=1) in the 60s machines,
  • the phase count is further hidden in "ABC" / "ABCDE", range(5), np.zeros((len(time), 5)), etc.

Having the same constants in several places makes the code prone to errors.

Activity

  1. haitao-ZCU commented on Oct 1, 2026

    @haitao-ZCU
    Collaborator

    @sadda
    I prefer to split this issue into two parts and propose two seperate PRs:

    • move phase_groups() to abatract parents class like compute_initpos() with parameterized n_phase and belt_offset
    • parameterize phase count

    with thses actions, each PR can be as small as possible.

  2. haitao-ZCU commented on Oct 1, 2026

    @haitao-ZCU
    Collaborator

    @sadda
    Hi, Lukas, with the parameterizations of n_phase, phase count, and so on, I found we can simplify the process of creating coils in 3f_36s and 5f_40s, e.g we can use general method to create them with loops and parameterizations instead of hard-coding.
    if possible, could you please create a new issue about this, and I'll do it.

  3. sadda commented on Oct 1, 2026

    @sadda
    CollaboratorAuthor

    @haitao-ZCU Agreed. Create the issue and then the corresponding PR.

  4. sadda commented on Oct 2, 2026

    @sadda
    CollaboratorAuthor

    This has been performed in #61 and #63.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions