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

new reactor class for negative triangularity reactor #55

Merged
merged 40 commits into from
Mar 9, 2022

Conversation

mateczentye
Copy link
Collaborator

@mateczentye mateczentye commented Jul 30, 2021

Proposed changes

Still under development - need to finish the class

  • adding setters and getters for attributes
  • adding PF coils and port cutters to support lists properly and cut the rest of geometry if intersecting
    - port cutter arguments take lists now with fully customizable locations by giving Z positions, and azimuthal angles along
    with the height and width of a rectangle port.
  • adding conditions for the tests to getters and setters to run with CI
  • general tidy up the code and add docstring
  • add tests

Types of changes

What types of changes does your code introduce to the Paramak?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactoring
  • Documentation Update (if none of the other choices apply)
  • New tests

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • Pep8 applied
  • Unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@shimwell shimwell marked this pull request as draft July 30, 2021 12:57
@shimwell
Copy link
Member

shimwell commented Jul 30, 2021

This is a new reactor class made by Mate during his SEPnet placement. This reactor supports negative triangularity by moving the divertors. While the BallReactor class allows negative plasma, it does not move the divertors to the tips of the plasma.

@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #55 (d868999) into develop (c2a595c) will decrease coverage by 0.03%.
The diff coverage is 89.80%.

❗ Current head d868999 differs from pull request most recent head 5401345. Consider uploading reports for the commit 5401345 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #55      +/-   ##
===========================================
- Coverage    97.57%   97.54%   -0.04%     
===========================================
  Files           76       77       +1     
  Lines         4703     7375    +2672     
===========================================
+ Hits          4589     7194    +2605     
- Misses         114      181      +67     
Impacted Files Coverage Δ
...ametric_reactors/negative_triangularity_reactor.py 89.78% <89.78%> (ø)
paramak/__init__.py 100.00% <100.00%> (ø)
paramak/parametric_components/blanket_fp.py 100.00% <0.00%> (ø)
paramak/parametric_components/hexagon_pin.py 100.00% <0.00%> (ø)
paramak/parametric_components/hollow_cube.py 100.00% <0.00%> (ø)
paramak/parametric_components/cutting_wedge.py 100.00% <0.00%> (ø)
paramak/parametric_components/divertor_ITER.py 100.00% <0.00%> (ø)
paramak/parametric_reactors/iter_paper_2020.py 100.00% <0.00%> (ø)
paramak/parametric_shapes/sweep_mixed_shape.py 100.00% <0.00%> (ø)
... and 68 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2a595c...5401345. Read the comment docs.

@shimwell
Copy link
Member

shimwell commented Oct 6, 2021

Hi @mateczentye welcome back. Just to let you know we have been adding input_varibles as a list to reactor classes. See this one for example

# adds self.input_variable_names from the Reactor class
self.input_variable_names = self.input_variable_names + [
'inner_bore_radial_thickness',
'inboard_tf_leg_radial_thickness',
'center_column_shield_radial_thickness',
'divertor_radial_thickness',
'inner_plasma_gap_radial_thickness',
'plasma_radial_thickness',
'outer_plasma_gap_radial_thickness',
'firstwall_radial_thickness',
'blanket_radial_thickness',
'blanket_rear_wall_radial_thickness',
'elongation',
'triangularity',
'plasma_gap_vertical_thickness',
'divertor_to_tf_gap_vertical_thickness',
'number_of_tf_coils',
'rear_blanket_to_tf_gap',
'pf_coil_radial_thicknesses',
'pf_coil_vertical_thicknesses',
'pf_coil_radial_position',
'pf_coil_vertical_position',
'pf_coil_case_thicknesses',
'outboard_tf_coil_radial_thickness',
'outboard_tf_coil_poloidal_thickness',
'divertor_position',
'rotation_angle',
]

@mateczentye
Copy link
Collaborator Author

Hi @mateczentye welcome back. Just to let you know we have been adding input_varibles as a list to reactor classes. See this one for example

# adds self.input_variable_names from the Reactor class
self.input_variable_names = self.input_variable_names + [
'inner_bore_radial_thickness',
'inboard_tf_leg_radial_thickness',
'center_column_shield_radial_thickness',
'divertor_radial_thickness',
'inner_plasma_gap_radial_thickness',
'plasma_radial_thickness',
'outer_plasma_gap_radial_thickness',
'firstwall_radial_thickness',
'blanket_radial_thickness',
'blanket_rear_wall_radial_thickness',
'elongation',
'triangularity',
'plasma_gap_vertical_thickness',
'divertor_to_tf_gap_vertical_thickness',
'number_of_tf_coils',
'rear_blanket_to_tf_gap',
'pf_coil_radial_thicknesses',
'pf_coil_vertical_thicknesses',
'pf_coil_radial_position',
'pf_coil_vertical_position',
'pf_coil_case_thicknesses',
'outboard_tf_coil_radial_thickness',
'outboard_tf_coil_poloidal_thickness',
'divertor_position',
'rotation_angle',
]

Thanks @shimwell! Finally got some time to do a bit of coding 😄. I have added the 'input_variables' and made some fixes will add tests this weekend.

@shimwell
Copy link
Member

looks like this reactor is getting closer, just wanted to mention the Value 'Optional' is unsubscriptable error in the code inspector report can be ignored. It is a bug with the version of pylint used

@shimwell
Copy link
Member

@mateczentye I am a bit unsure about these __init__.py files in the test folders. Just wondering if there is any motivation for these new files or can they be removed.

@mateczentye
Copy link
Collaborator Author

@mateczentye I am a bit unsure about these __init__.py files in the test folders. Just wondering if there is any motivation for these new files or can they be removed.

@shimwell sorry been offline for a few weeks as life happened. Not entirely sure how and what I did there, so apologies they shouldn't be there!

@shimwell
Copy link
Member

shimwell commented Dec 1, 2021

I've returned some of the files removed. just to keep this PR clean. There are now only 3 files changed by this PR.

I'm might still remove those files, but I will do it in a separate PR with a different scope

@mateczentye
Copy link
Collaborator Author

export_1_diag

@mateczentye
Copy link
Collaborator Author

image

image

@shimwell
Copy link
Member

shimwell commented Dec 1, 2021

Looking good, I wonder if it is worth moving the location PF the of coils so that they don't cut the TF coils

@shimwell
Copy link
Member

shimwell commented Mar 9, 2022

Thanks for all the nice work here @mateczentye
Merging in and I can tidy up the images later.
Thanks

@shimwell shimwell marked this pull request as ready for review March 9, 2022 21:54
@shimwell shimwell merged commit 29fc03b into develop Mar 9, 2022
@shimwell shimwell deleted the negative_triangularity_MC branch March 16, 2022 10:13
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.

2 participants