4233 global power imbalance output#4417
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4417 +/- ##
==========================================
- Coverage 48.87% 48.74% -0.14%
==========================================
Files 151 151
Lines 29424 29505 +81
==========================================
Hits 14381 14381
- Misses 15043 15124 +81 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
timothy-nunn
left a comment
There was a problem hiding this comment.
Just some suggestions from a quick code review. Will check the calculations in another review.
| if self.data.physics.i_plasma_ignited == 1: | ||
| po.ocmmnt(self.outfile, " (Injected power only used for start-up phase)") | ||
|
|
||
| # Global power imbalance output #4233 ######################################## |
There was a problem hiding this comment.
| # Global power imbalance output #4233 ######################################## |
There was a problem hiding this comment.
Row of # removed. Comment unchanged
| po.ovarre( | ||
| self.outfile, | ||
| "Plasma power imbalance (MW)", | ||
| "(p_plasma_out - p_plasma_in)", |
There was a problem hiding this comment.
| "(p_plasma_out - p_plasma_in)", | |
| "(p_plasma_out_subtract_p_plasma_in)", |
I don't think this will be able to be indexed from the MFILE properly
There was a problem hiding this comment.
All these have been replaced by new variables:
"(p_plasma_imbalance_mw)",
etc
| if abs(p_plasma_out - p_plasma_in) > 0.1: | ||
| logger.error("Plasma power imbalance > 0.1 MW") | ||
|
|
||
| #################################################### |
There was a problem hiding this comment.
| #################################################### |
I know these still exist in the code but I try and keep them at a minimum
| po.ovarre( | ||
| self.outfile, | ||
| "Electric power imbalance (MW)", | ||
| "()", |
There was a problem hiding this comment.
Can you assign this an output name otherwise I suspect it will cause issues in the MFILE parsing at some point
| po.ovarre( | ||
| self.outfile, | ||
| "Power plant overall imbalance (MW)", | ||
| "(p_plant_in_mw - p_plant_out_mw)", |
There was a problem hiding this comment.
| "(p_plant_in_mw - p_plant_out_mw)", | |
| "(p_plant_in_mw_subtract_p_plant_out_mw)", |
timothy-nunn
left a comment
There was a problem hiding this comment.
A lot of these variables are already being output in the same routine/in other models. I don't see a reason to do this twice?
| po.ovarre( | ||
| self.outfile, | ||
| "Net power transported by electrons (MW)", | ||
| "(p_electron_transport_loss_mw)", | ||
| self.data.physics.p_electron_transport_loss_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Net power transported by ions (MW)", | ||
| "(p_ion_transport_loss_mw)", | ||
| self.data.physics.p_ion_transport_loss_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
These variables are already output above
| po.ovarre( | |
| self.outfile, | |
| "Net power transported by electrons (MW)", | |
| "(p_electron_transport_loss_mw)", | |
| self.data.physics.p_electron_transport_loss_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Net power transported by ions (MW)", | |
| "(p_ion_transport_loss_mw)", | |
| self.data.physics.p_ion_transport_loss_mw, | |
| "OP ", | |
| ) |
There was a problem hiding this comment.
Indeed. Nevertheless, we need to check that the powers balance, and there is no easy way to check that from the existing outputs. A certain amount of redundancy is not a big deal. It may be possible to slim down the rest of the output file - I can look at that, but that would be another issue.
In theory one could hide the output if it balances, and display it only if there is a discrepancy, but I don't really see the benefit.
There was a problem hiding this comment.
I have removed a couple of duplications.
The electric power requirements are output in a somewhat redundant way. I will look at this in a separate issue.
| po.ovarre( | ||
| self.outfile, | ||
| "Net loss by synchrotron radiation (MW)", | ||
| "(p_plasma_sync_mw)", | ||
| self.data.physics.p_plasma_sync_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Already output above
| po.ovarre( | |
| self.outfile, | |
| "Net loss by synchrotron radiation (MW)", | |
| "(p_plasma_sync_mw)", | |
| self.data.physics.p_plasma_sync_mw, | |
| "OP ", | |
| ) |
| po.ovarre( | ||
| self.outfile, | ||
| "Alpha power (MW)", | ||
| "(p_alpha_total_mw)", | ||
| self.data.physics.p_alpha_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Power from p, 3He, T products of DD and/or D-He3 fusion (MW)", | ||
| "(p_non_alpha_charged_mw)", | ||
| self.data.physics.p_non_alpha_charged_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Injected power (MW)", | ||
| "(p_hcd_injected_total_mw)", | ||
| self.data.current_drive.p_hcd_injected_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Ohmic heating (MW)", | ||
| "(p_plasma_ohmic_mw)", | ||
| self.data.physics.p_plasma_ohmic_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
| po.ovarre( | |
| self.outfile, | |
| "Alpha power (MW)", | |
| "(p_alpha_total_mw)", | |
| self.data.physics.p_alpha_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Power from p, 3He, T products of DD and/or D-He3 fusion (MW)", | |
| "(p_non_alpha_charged_mw)", | |
| self.data.physics.p_non_alpha_charged_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Injected power (MW)", | |
| "(p_hcd_injected_total_mw)", | |
| self.data.current_drive.p_hcd_injected_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Ohmic heating (MW)", | |
| "(p_plasma_ohmic_mw)", | |
| self.data.physics.p_plasma_ohmic_mw, | |
| "OP ", | |
| ) |
Already output above
| po.ovarre( | ||
| self.outfile, | ||
| "Fusion power (MW)", | ||
| "(p_fusion_total_mw)", | ||
| self.data.physics.p_fusion_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Energy multiplication in blanket and shield (MW)", | ||
| "(p_blkt_multiplication_mw)", | ||
| self.data.fwbs.p_blkt_multiplication_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Injected power (MW)", | ||
| "(p_hcd_injected_total_mw)", | ||
| self.data.current_drive.p_hcd_injected_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Ohmic heating (MW)", | ||
| "(p_plasma_ohmic_mw)", | ||
| self.data.physics.p_plasma_ohmic_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
| po.ovarre( | |
| self.outfile, | |
| "Fusion power (MW)", | |
| "(p_fusion_total_mw)", | |
| self.data.physics.p_fusion_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Energy multiplication in blanket and shield (MW)", | |
| "(p_blkt_multiplication_mw)", | |
| self.data.fwbs.p_blkt_multiplication_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Injected power (MW)", | |
| "(p_hcd_injected_total_mw)", | |
| self.data.current_drive.p_hcd_injected_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Ohmic heating (MW)", | |
| "(p_plasma_ohmic_mw)", | |
| self.data.physics.p_plasma_ohmic_mw, | |
| "OP ", | |
| ) |
Already output above
| po.ovarre( | ||
| self.outfile, | ||
| "Power deposited by pump in coolant for FW and blanket circuit (MW)", | ||
| "(p_fw_blkt_coolant_pump_mw)", | ||
| self.data.primary_pumping.p_fw_blkt_coolant_pump_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Power deposited by pump in coolant for divertor circuit (MW)", | ||
| "(p_div_coolant_pump_mw)", | ||
| self.data.heat_transport.p_div_coolant_pump_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Already output in another module
| po.ovarre( | |
| self.outfile, | |
| "Power deposited by pump in coolant for FW and blanket circuit (MW)", | |
| "(p_fw_blkt_coolant_pump_mw)", | |
| self.data.primary_pumping.p_fw_blkt_coolant_pump_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Power deposited by pump in coolant for divertor circuit (MW)", | |
| "(p_div_coolant_pump_mw)", | |
| self.data.heat_transport.p_div_coolant_pump_mw, | |
| "OP ", | |
| ) |
| po.ovarre( | ||
| self.outfile, | ||
| "Net electric (MW)", | ||
| "(p_plant_electric_net_mw)", | ||
| self.data.heat_transport.p_plant_electric_net_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "TF coils electric power (MW)", | ||
| "(p_tf_electric_supplies_mw)", | ||
| self.data.heat_transport.p_tf_electric_supplies_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "PF coils electric power (MW)", | ||
| "(p_pf_electric_supplies_mw)", | ||
| self.data.pf_coil.p_pf_electric_supplies_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Heating and current drive electric power (MW)", | ||
| "(p_hcd_electric_total_mw)", | ||
| self.data.heat_transport.p_hcd_electric_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Primary coolant pump electric power (MW)", | ||
| "(p_coolant_pump_elec_total_mw)", | ||
| self.data.heat_transport.p_coolant_pump_elec_total_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Already being output in another model
| po.ovarre( | |
| self.outfile, | |
| "Net electric (MW)", | |
| "(p_plant_electric_net_mw)", | |
| self.data.heat_transport.p_plant_electric_net_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "TF coils electric power (MW)", | |
| "(p_tf_electric_supplies_mw)", | |
| self.data.heat_transport.p_tf_electric_supplies_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "PF coils electric power (MW)", | |
| "(p_pf_electric_supplies_mw)", | |
| self.data.pf_coil.p_pf_electric_supplies_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Heating and current drive electric power (MW)", | |
| "(p_hcd_electric_total_mw)", | |
| self.data.heat_transport.p_hcd_electric_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Primary coolant pump electric power (MW)", | |
| "(p_coolant_pump_elec_total_mw)", | |
| self.data.heat_transport.p_coolant_pump_elec_total_mw, | |
| "OP ", | |
| ) |
| po.ovarre( | ||
| self.outfile, | ||
| "Vacuum pumps electric power (MW)", | ||
| "(vachtmw)", | ||
| self.data.heat_transport.vachtmw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Cryoplant electric power (MW)", | ||
| "(p_cryo_plant_electric_mw)", | ||
| self.data.heat_transport.p_cryo_plant_electric_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
| po.ovarre( | |
| self.outfile, | |
| "Vacuum pumps electric power (MW)", | |
| "(vachtmw)", | |
| self.data.heat_transport.vachtmw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Cryoplant electric power (MW)", | |
| "(p_cryo_plant_electric_mw)", | |
| self.data.heat_transport.p_cryo_plant_electric_mw, | |
| "OP ", | |
| ) |
Already being output
| po.ovarre( | ||
| self.outfile, | ||
| "Gross electric output (MW)", | ||
| "(p_plant_electric_gross_mw)", | ||
| self.data.heat_transport.p_plant_electric_gross_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
| po.ovarre( | |
| self.outfile, | |
| "Gross electric output (MW)", | |
| "(p_plant_electric_gross_mw)", | |
| self.data.heat_transport.p_plant_electric_gross_mw, | |
| "OP ", | |
| ) |
Already being output
| po.ovarre( | ||
| self.outfile, | ||
| "Fusion power (MW)", | ||
| "(p_fusion_total_mw)", | ||
| self.data.physics.p_fusion_total_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Energy multiplication in blanket and shield (MW)", | ||
| "(p_blkt_multiplication_mw)", | ||
| self.data.fwbs.p_blkt_multiplication_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
| po.ovarre( | |
| self.outfile, | |
| "Fusion power (MW)", | |
| "(p_fusion_total_mw)", | |
| self.data.physics.p_fusion_total_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Energy multiplication in blanket and shield (MW)", | |
| "(p_blkt_multiplication_mw)", | |
| self.data.fwbs.p_blkt_multiplication_mw, | |
| "OP ", | |
| ) |
Already being output
| po.ovarre( | ||
| self.outfile, | ||
| "Net electric (MW)", | ||
| "(p_plant_electric_net_mw)", | ||
| self.data.heat_transport.p_plant_electric_net_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Heat rejected by main power conversion circuit (MW)", | ||
| "(p_turbine_loss_mw)", | ||
| self.data.power.p_turbine_loss_mw, | ||
| "OP ", | ||
| ) | ||
| po.ovarre( | ||
| self.outfile, | ||
| "Heat rejected by other circuits (secondary heat) (MW)", | ||
| "(p_plant_secondary_heat_mw)", | ||
| self.data.heat_transport.p_plant_secondary_heat_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Already output in another model
| po.ovarre( | |
| self.outfile, | |
| "Net electric (MW)", | |
| "(p_plant_electric_net_mw)", | |
| self.data.heat_transport.p_plant_electric_net_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Heat rejected by main power conversion circuit (MW)", | |
| "(p_turbine_loss_mw)", | |
| self.data.power.p_turbine_loss_mw, | |
| "OP ", | |
| ) | |
| po.ovarre( | |
| self.outfile, | |
| "Heat rejected by other circuits (secondary heat) (MW)", | |
| "(p_plant_secondary_heat_mw)", | |
| self.data.heat_transport.p_plant_secondary_heat_mw, | |
| "OP ", | |
| ) |
…nks. Some duplicate outputs removed from power.py and hcpb.py
|
@mkovari Can I ask that this is adjusted to use the |
I have long been concerned about the ignited option. I have never used it and I am not sure we need it. I am not clear why this is here, for example: # If the device is not ignited, add the injected auxiliary power
if PlasmaIgnitionModel(i_plasma_ignited) == PlasmaIgnitionModel.NON_IGNITED:
p_plasma_loss_mw += p_hcd_injected_total_mwThe power should balance whether the user thinks the plasma is ignited or not. Leaving this aside, could you clarify how and where I need to use this switch in the power accounting checks? Thanks @chris-ashe |
chris-ashe
left a comment
There was a problem hiding this comment.
Few bits about readability and outputting. Some of the calculations are double counting. I will probably put a few spinout PR's out to increase robustness and prevent the need for multiple redundant declaration
| self.data.current_drive.p_hcd_injected_electrons_mw, | ||
| "OP ", | ||
| ) | ||
| if self.data.physics.i_plasma_ignited == 1: |
There was a problem hiding this comment.
For the ignition enums from what I have seen the ignited case will never apply any HCD power so it is never added. In reality the number should just be set to zero. See the enum instance in physics_variables.py to apply the correct enum test instead of the raw number matching
There was a problem hiding this comment.
OK, so GitHub is showing the code above as green with + signs, indicating it has been added. In fact I didn't touch it, except for removing a couple of now-redundant outputs. My contributions start from line 2288: # Global power imbalance output #4233
In fact in the latest version of main this has already been fixed, by you.
No action (other than merging main back in).
| po.ocmmnt(self.outfile, " (Injected power only used for start-up phase)") | ||
|
|
||
| # Global power imbalance output #4233 | ||
| po.oheadr(self.outfile, "Power accounting") |
There was a problem hiding this comment.
Each output should be a separate function and not nested inside of outplas() 3 of the output sections related to the total plant should be put in power.py
There was a problem hiding this comment.
I don't see any benefit in making the code more complicated by creating new functions.
I agree that the last three sections could go into power.py. However, there is also something to be said for putting these accounting checks together so it is quick and easy to review them all.
I propose no action.
| ) | ||
| po.ocmmnt( | ||
| self.outfile, | ||
| "See Figure in https://ukaea.github.io/PROCESS/physics-models/plasma_power_balance", |
There was a problem hiding this comment.
The referenced figure is very out of date and none of the variables match, can this please be updated
There was a problem hiding this comment.
Yes - for another day. No action.
| + self.data.physics.p_fw_alpha_mw | ||
| ) | ||
| p_impurity_mw = ( | ||
| self.data.physics.p_plasma_inner_rad_mw |
There was a problem hiding this comment.
Note that the p_plasma_inner_rad_mw contains the synchrotron contribution as we always assume it to come from just the core region. The variable p_plasma_rad_mw contains all radiation power
There was a problem hiding this comment.
Indeed it does.
I will fix this. TODO
Oddly, confinement time.py has these lines:
p_plasma_loss_mw
+ self.data.physics.pden_plasma_sync_mw * vol_plasma
+ self.data.physics.p_plasma_inner_rad_mwwhich does seem to be double counting.
| + p_impurity_mw | ||
| ) | ||
|
|
||
| p_plasma_in = ( |
There was a problem hiding this comment.
The variable p_plasma_heating_total_mw contains the in-situ calculated value for the total heating power. The function calculate_total_plasma_heating_power() also exists
There was a problem hiding this comment.
Indeed. This section functions as a kind of independent audit of the calculations elsewhere. Duplication of effort - yes. Worth doing - also yes, in my opinion.
The quantities defined here are not intended to be used anywhere else.
| p_plasma_imbalance_mw = p_plasma_out - p_plasma_in | ||
| po.oshead(self.outfile, "Plasma power balance across separatrix") | ||
| po.ovarre( | ||
| self.outfile, |
There was a problem hiding this comment.
Using po.ocmmnt() here to create subheadings for "Heating:" and "Losses:" would increase readability
There was a problem hiding this comment.
I will see how this looks. TODO
| "(p_impurity_mw)", | ||
| p_impurity_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Using po.oblnkl(self.outfile) would be good to separate the total value from the summed terms for readability
There was a problem hiding this comment.
I will see how this looks. TODO
There was a problem hiding this comment.
Done. I didn't separate the total but I put it in caps.
| + self.data.current_drive.p_hcd_injected_total_mw | ||
| + self.data.physics.p_plasma_ohmic_mw | ||
| ) | ||
| p_plasma_imbalance_mw = p_plasma_out - p_plasma_in |
There was a problem hiding this comment.
Would be better hear to do the heating powers minus the loss power so that a returned negative value means a net loss, which makes sense
| "(p_plasma_in)", | ||
| p_plasma_in, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Using po.oblnkl(self.outfile) would be good to separate the total value from the summed terms for readability
There was a problem hiding this comment.
Done. I put total in caps.
| "(p_plasma_ohmic_mw)", | ||
| self.data.physics.p_plasma_ohmic_mw, | ||
| "OP ", | ||
| ) |
There was a problem hiding this comment.
Using po.oblnkl(self.outfile) would be good to separate the total value from the summed terms for readability
There was a problem hiding this comment.
I will see how this looks. TODO
There was a problem hiding this comment.
Done. I didn't put in a blank line but I put it in caps.
This implements issue #4233. I have restored the power accounting section in the output files.
Note the powers do not balance - see issue for details.
The only changes are additional outputs.
I have merged main into this branch, but's a long time since I have made a merge request - let me know if there are any problems. I have no idea how to update the outputs of the regression tests, or if I need to do this.
I will create new issues to resolve the imbalances. Those will need to be looked at by Chris and others - for the moment I suggest any of the people on the maintainer list are sufficient.