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.

Sign up for GitHub

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

Jump to bottom

Add toroidal and poloidal variation of component thickness #9

Merged
merged 3 commits into from
Nov 4, 2023
Merged

Add toroidal and poloidal variation of component thickness #9

merged 3 commits into from
Nov 4, 2023

Conversation

connoramoreno
Copy link
Collaborator

Adds ability for user to specify toroidal and poloidal variation of component thicknesses. Users specify component thicknesses at points in the toroidal and poloidal domain, and thickness values are linearly interpolated between these points to provide continuous variation.

gonuke
gonuke requested changes Sep 17, 2023
View reviewed changes
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a couple of initial requests, but I think it will be good to discuss data structure choices for the now quite complex radial build data in a S/W meeting

parametric_stellarator.py Outdated Show resolved Hide resolved
gonuke
gonuke requested changes Oct 18, 2023
View reviewed changes
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small comment on the readability and robustness of your angle expansion.

Aside: I think we are at a tipping point for wanting to make this more object oriented!

magnet_coils.py Outdated Show resolved Hide resolved
parastell.py Outdated Show resolved Hide resolved
parastell.py Outdated Show resolved Hide resolved
@connoramoreno connoramoreno mentioned this pull request Oct 21, 2023
Closed
gonuke
gonuke requested changes Oct 23, 2023
View reviewed changes
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One little change recommended here

parastell.py Show resolved Hide resolved
@gonuke
Copy link
Member

gonuke commented Oct 23, 2023 •
edited
Loading

I think this needs a rebase because of #11 ?

@connoramoreno connoramoreno force-pushed the continuous_radial_build branch from 69b69e9 to e522138 Compare November 2, 2023 15:32
@connoramoreno
Copy link
Collaborator Author

Rebased on #11

gonuke
gonuke approved these changes Nov 4, 2023
View reviewed changes
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@gonuke gonuke merged commit ca53abd into svalinn:main Nov 4, 2023
connoramoreno pushed a commit that referenced this pull request Sep 10, 2024
@gonuke
Merge pull request #9 from connoramoreno/continuous_radial_build
c96d9c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gonuke gonuke gonuke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@connoramoreno @gonuke