Skip to content

TF_WP #3082

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

Closed
wants to merge 17 commits into from
Closed

TF_WP #3082

wants to merge 17 commits into from

Conversation

ivanmaione
Copy link
Contributor

Linked Issues

Closes #138

Description

Interface Changes

Checklist

I confirm that I have completed the following checks:

  • Tests run locally and pass pytest tests --reactor
  • Code quality checks run locally and pass pre-commit run --from-ref develop --to-ref HEAD
  • Documentation built locally and checked sphinx-build -W documentation/source documentation/build

@ivanmaione ivanmaione requested a review from a team as a code owner March 9, 2024 07:04
Copy link

sonarqubecloud bot commented Mar 9, 2024

Quality Gate Passed Quality Gate passed

Issues
132 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
6.1% Duplication on New Code

See analysis details on SonarCloud

@ivanmaione
Copy link
Contributor Author

@CoronelBuendia I cannot rebase the previous branch in the "correct" way. The easy solution was to create a new branch starting from develop. If it is ok, I will close the other PR (just labelled as "obsolete" for the moment). Sorry for the confusion.

@CoronelBuendia CoronelBuendia changed the base branch from develop to feature/new_magnet_module March 10, 2024 19:23
@CoronelBuendia CoronelBuendia requested review from a team as code owners March 10, 2024 19:24
@CoronelBuendia CoronelBuendia changed the base branch from feature/new_magnet_module to develop March 10, 2024 19:24
@je-cook je-cook marked this pull request as draft March 11, 2024 13:18
Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 1098 lines in your changes missing coverage. Please review.

Project coverage is 73.47%. Comparing base (00e9f87) to head (9dc4d5a).
Report is 180 commits behind head on develop.

Files with missing lines Patch % Lines
bluemira/magnets/cable.py 0.00% 257 Missing ⚠️
bluemira/magnets/case_tf.py 0.00% 179 Missing ⚠️
bluemira/magnets/fatigue.py 0.00% 157 Missing ⚠️
bluemira/magnets/strand.py 0.00% 157 Missing ⚠️
bluemira/magnets/conductor.py 0.00% 154 Missing ⚠️
bluemira/magnets/materials.py 0.00% 103 Missing ⚠️
bluemira/magnets/winding_pack.py 0.00% 51 Missing ⚠️
bluemira/magnets/utils.py 0.00% 40 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3082      +/-   ##
===========================================
- Coverage    74.27%   73.47%   -0.80%     
===========================================
  Files          242      238       -4     
  Lines        26825    27861    +1036     
===========================================
+ Hits         19923    20472     +549     
- Misses        6902     7389     +487     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@je-cook je-cook force-pushed the develop branch 5 times, most recently from 5a50acb to 7574352 Compare May 15, 2024 08:13
@je-cook je-cook removed request for a team May 29, 2024 15:28
@je-cook je-cook mentioned this pull request Jun 3, 2024
6 tasks
Copy link

sonarqubecloud bot commented Jun 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Copy link
Contributor

@CoronelBuendia CoronelBuendia left a comment

Choose a reason for hiding this comment

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

Hi @ivanmaione, sorry I don't have much time at the moment, so this is just an intermediate review... I will try and get back to it soon, but thought I'd get some comments in.

"""Material density [kg/m³]"""
return 8960

def erho(self, T: float, B: float, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

I picked this equation to review at random (well, because it looked more complicated). I am not able to retrace its origins entirely. From the reference in the docstring I can find parts of the below from equations 8-1 and 8-7, but there are some discrepancies from what I can tell, or perhaps just some undocumented values that differ from those in the reference (e.g. rho_c in 8-1).

Comment on lines +271 to +281
# superconducting parameters for the calculation of Ic
c_ = 1.0
Ca1 = 50.06
Ca2 = 0.00
eps_0a = 0.00312
eps_m = -0.00059
Bc20max = 33.24
Tc0max = 16.34
p = 0.593
q = 2.156
C = 83075 * 1e6
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a general parameterisation, that does not represent all Nb3Sn strands. We can separate the data from the parameterisation very easily in this case.

Comment on lines +346 to +357
n = 1.7
Bc20_T = 15.19
Tc0_K = 8.907
C0 = 3.00e04
C1 = 0.45
a1 = 3.2
b1 = 2.43
C2 = 0.55
a2 = 0.65
b2 = 2
g1 = 1.8
g2 = 1.8
Copy link
Contributor

Choose a reason for hiding this comment

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

SImilar here

Comment on lines +321 to +339
T_ = T + T_margin # noqa: N806
int_eps = -strain / 100
eps_sh = self.Ca2 * self.eps_0a / (np.sqrt(self.Ca1 ** 2 - self.Ca2 ** 2))
s_eps = 1 + (
self.Ca1
* (
np.sqrt(eps_sh ** 2 + self.eps_0a ** 2)
- np.sqrt((int_eps - eps_sh) ** 2 + self.eps_0a ** 2)
)
- self.Ca2 * int_eps
) / (1 - self.Ca1 * self.eps_0a)
Bc0_eps = self.Bc20max * s_eps
Tc0_eps = self.Tc0max * (s_eps) ** (1 / 3) # noqa: N806
t = T_ / Tc0_eps
BcT_eps = Bc0_eps * (1 - t ** (1.52))
b = B / BcT_eps
hT = (1 - t ** (1.52)) * (1 - t ** 2) # noqa: N806
fPb = (b ** self.p) * (1 - b) ** self.q # noqa: N806
return self.c_ * (self.C / B) * s_eps * fPb * hT
Copy link
Contributor

Choose a reason for hiding this comment

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

From my own implementations of these superconducting parameterisations, it is usually worthwhile to protect against NaNs and infinity. See PROCESS for an example of how to treat this.

"""Electrical Resistivity"""
return 1e6

def cp_v(self, **kwargs): # noqa: ARG002
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential for some confusion here. Is this the volumetric heat capacity or the specific heat capacity? So extensive of intensive? I think in general (and something that I regret a little with the present materials implementation in bluemira) that it would be nice to have full_word_names for these properties, even if sometimes they will look horrid. Saves on potential confusion of this nature. I also find the signature (temperature: float, field: Union[None, float], strain: Union[None, float]) perhaps a little easier to read.

@je-cook je-cook changed the title WIP: TF_WP TF_WP Nov 6, 2024
@ivanmaione ivanmaione closed this May 12, 2025
@je-cook je-cook deleted the ivanmaione/TF_WP_clean branch May 13, 2025 08:02
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.

Winding pack design optimisations
2 participants