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

Allow coil filaments to have variable number of points #125

Closed

Conversation

paulromano
Copy link
Collaborator

Currently, the logic for building magnet coils assumes that all filaments in the coils file have an equal number of points. When trying to use parastell on a coils file for W7-X, I found that the filaments don't have the same length (namely, non-planar and planar coils for W7-X have a different number of points). This PR updates the logic around storing filaments data and computing an average radial distance to accommodate having a variable number of points per filament.

@Edgar-21
Copy link
Contributor

This test is likely failing due to the PR coming from a branch on a fork, rather than this repo. Tests are passing locally for me.

@paulromano
Copy link
Collaborator Author

Given that the CI for both this and #126 is failing at the same spot (test_rectangular_magnets, which starts off by initializing Cubit), it does seem like it is related to the fact that the PR is coming from a fork. I'm happy to push these branches to svalinn if you guys are OK with that (not sure who makes that decision @connoramoreno? @gonuke?), although it would be good to figure out how to get PRs from forks working so that other external contributors don't run into the same thing.

@connoramoreno
Copy link
Collaborator

@paulromano I'm fine with you pushing directly to Svalinn as long as @gonuke doesn't mind. Although I agree that we should get PRs from forks working...

@gonuke
Copy link
Member

gonuke commented Jul 10, 2024

Given that the CI for both this and #126 is failing at the same spot (test_rectangular_magnets, which starts off by initializing Cubit), it does seem like it is related to the fact that the PR is coming from a fork. I'm happy to push these branches to svalinn if you guys are OK with that (not sure who makes that decision @connoramoreno? @gonuke?), although it would be good to figure out how to get PRs from forks working so that other external contributors don't run into the same thing.

The reason we can't PR from a fork is that we have a GITHUB secret for accessing the Cubit license server. Since we rely on Cubit, it's probably not something we can resolve. It's easier to grant folks access to our repo than to share that license server key/secret.

@gonuke
Copy link
Member

gonuke commented Jul 10, 2024

I've invited @paulromano to this repo as a maintainer

@paulromano paulromano closed this Jul 11, 2024
@paulromano paulromano deleted the variable-length-filaments branch July 11, 2024 00:54
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