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

Remove shift and rotmat attributes from Curve class #1418

Open
ddudt opened this issue Dec 2, 2024 · 4 comments
Open

Remove shift and rotmat attributes from Curve class #1418

ddudt opened this issue Dec 2, 2024 · 4 comments
Assignees
Labels
coil stuff relating to coils and coil optimization P2 Medium Priority, not urgent but should be on the near-term agend

Comments

@ddudt
Copy link
Collaborator

ddudt commented Dec 2, 2024

These attributes are redundant with the other optimizable parameters for each curve type, and it is getting cumbersome to maintain the special logic for them. They were intended to make it easier to initialize coil geometries, but this might be obsolete now with other utility functions we have added. These attributes are mostly hidden from the user, so removing them would not change the UI.

@ddudt ddudt added coil stuff relating to coils and coil optimization P2 Medium Priority, not urgent but should be on the near-term agend labels Dec 2, 2024
@ddudt ddudt self-assigned this Dec 2, 2024
@dpanici
Copy link
Collaborator

dpanici commented Dec 4, 2024

Why again can we not just have these as attribute and passed in as transforms?

@f0uriest
Copy link
Member

f0uriest commented Dec 4, 2024

Because for a coilset, we only have one set of transforms, but the shift and rotmat can be different for each coil, so they have to be in params

@dpanici
Copy link
Collaborator

dpanici commented Dec 9, 2024

Keep as params for just FourierRZCoil (and FourierRZCurve)

@dpanici
Copy link
Collaborator

dpanici commented Dec 9, 2024

But we should just get rid of it for all curves

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coil stuff relating to coils and coil optimization P2 Medium Priority, not urgent but should be on the near-term agend
Projects
None yet
Development

No branches or pull requests

3 participants