Improve robustness of t_zero_margin calculation with brentq fallback#4018
Improve robustness of t_zero_margin calculation with brentq fallback#4018chris-ashe wants to merge 2 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4018 +/- ##
=======================================
Coverage 46.30% 46.31%
=======================================
Files 123 123
Lines 28961 28974 +13
=======================================
+ Hits 13411 13419 +8
- Misses 15550 15555 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I still need to think about this and test it a bit more. This doesn't fix the problem, but I am not so sure there is a solution: we are at a garbage point in the parameter space where a positive solution does not exist.
This PR will stop PROCESS from crashing and instead it will (probably) fail to converge.
The only thing I need to convince myself of is that -1 is an acceptable fallback. Could this -1 end up in the availability model or a constraint, cause that model to be wrong, but we somehow converge?
timothy-nunn
left a comment
There was a problem hiding this comment.
I'm still thinking so no rush with these changes. I think this mixture of local/global is redundant and might even cause the two to be different at points in the execution.
process/superconducting_tf_coil.py
Outdated
| full_output=True, | ||
| disp=True, | ||
| ) | ||
| temp_tf_superconductor_margin = -1.0e0 # Default value in case of exception |
There was a problem hiding this comment.
| temp_tf_superconductor_margin = -1.0e0 # Default value in case of exception | |
Remove this default and use the variable tfcoil_variables.temp_margin, I think having the local variable is redundant
process/superconducting_tf_coil.py
Outdated
| logger.error( | ||
| """Negative TFC temperature margin | ||
| f"""Negative TFC temperature margin | ||
| temp_tf_superconductor_margin: {temp_tf_superconductor_margin} |
There was a problem hiding this comment.
| temp_tf_superconductor_margin: {temp_tf_superconductor_margin} | |
| temp_tf_superconductor_margin: {tfcoil_variables.temp_margin} |
process/superconducting_tf_coil.py
Outdated
| temp_tf_superconductor_margin = t_zero_margin - temp_tf_coolant_peak_field | ||
| tfcoil_variables.temp_margin = temp_tf_superconductor_margin | ||
|
|
||
| if temp_tf_superconductor_margin <= 0.0e0: |
There was a problem hiding this comment.
| if temp_tf_superconductor_margin <= 0.0e0: | |
| if tfcoil_variables.temp_margin <= 0.0e0: |
process/superconducting_tf_coil.py
Outdated
There was a problem hiding this comment.
| return tfcoil_variables.temp_margin |
861a8a7 to
d667b8b
Compare
|
Hi @mkovari, could I request your review here. Chris has added two methods to try and find the root using both Here is what the approximation looks like between temp=$5$ and the root from the linear approximation: |


Description
Checklist
I confirm that I have completed the following checks: