Fixed negativity in sqrt in costs.py by np.clip.

[OceanNuclear]

Description

Patches away the RuntimeWarning caused by sqrt(p_plant_electric_net_mw/1200), where p_plant_electric_net_mw may cross over to negative values briefly during the optimization process (but is expected to always converge on a positive value).

Checklist

I confirm that I have completed the following checks:

• My changes follow the PROCESS style guide
• I have justified any large differences in the regression tests caused by this pull request in the comments.
• I have added new tests where appropriate for the changes I have made. (N/A)
• If I have had to change any existing unit or integration tests, I have justified this change in the pull request comments.
• If I have made documentation changes, I have checked they render correctly. (N/A)
• I have added documentation for my change, if appropriate. (N/A)

[OceanNuclear]

Closes #4024

[codecov-commenter]

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.50%. Comparing base (ac78fdb) to head (0f6eb3a).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
process/costs.py	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4065      +/-   ##
==========================================
- Coverage   46.60%   46.50%   -0.11%     
==========================================
  Files         122      124       +2     
  Lines       28711    28961     +250     
==========================================
+ Hits        13381    13468      +87     
- Misses      15330    15493     +163     

☔ 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.

[timothy-nunn]

I've spoken with quite a few people about this, some of these comments and concerns are written in the original issue.

The model is clearly flawed because it uses net electric to try and estimate the O&M cost. This is done because all O&M models in the literature are fits on data from existing fission/gas plants. I suspect net electric is used here as a loose proxy for plant complexity.

My suggestion is to accept and merge this PR because it will fix the negative square root, and does no harm: the cost model will produce a NaN if given a negative number and so producing a 0 instead is no worse.

An issue should be created where we can discuss and plan how to produce more realistic O&M costs that rely on something a bit more meaningful that net electric.

@geograham @j-a-foster how does this sound to you? If we all agree, I will merge this PR, create an issue and we can discuss our concerns further there and plan if/how we want to fix it.

[timothy-nunn]

Can we add a warning when the net electric is negative and the value will be clipped?

[geograham]

I've spoken with quite a few people about this, some of these comments and concerns are written in the original issue.

The model is clearly flawed because it uses net electric to try and estimate the O&M cost. This is done because all O&M models in the literature are fits on data from existing fission/gas plants. I suspect net electric is used here as a loose proxy for plant complexity.

My suggestion is to accept and merge this PR because it will fix the negative square root, and does no harm: the cost model will produce a NaN if given a negative number and so producing a 0 instead is no worse.

An issue should be created where we can discuss and plan how to produce more realistic O&M costs that rely on something a bit more meaningful that net electric.

@geograham @j-a-foster how does this sound to you? If we all agree, I will merge this PR, create an issue and we can discuss our concerns further there and plan if/how we want to fix it.

I think this makes sense - a warning about clipped net electric in the meantime is also a good idea.

[OceanNuclear]

Funnily enough that might bring us almost full circle back to issue #4024; but in some way progress is still made by identifying the bug. @timothy-nunn If you're happy with @geograham 's solution then I'll add a warning back in.

[timothy-nunn]

Funnily enough that might bring us almost full circle back to issue #4024; but in some way progress is still made by identifying the bug. @timothy-nunn If you're happy with @geograham 's solution then I'll add a warning back in.

Hi @OceanNuclear, I think lets add the warning. This warning won't be a cryptic 'divide by 0' warning and it will reported as a model warning (logger.warning) and so it will be a lot more useful than the previous divide by 0 warning.

[timothy-nunn]

Just improving the clarity of the error message

[timothy-nunn]

The /1200 was in the square root before