architecture-building-systems / CityEnergyAnalyst

The City Energy Analyst (CEA)
https://www.cityenergyanalyst.com/
MIT License
196 stars 62 forks source link

fixing operation result files #3598

Closed MatNif closed 3 months ago

MatNif commented 4 months ago

Due to an index mismatch between the different pandas.Series that make up the operational performance profiles of the supply systems, the resulting data frame contained a lot of null values. This change should fix that.

To check if the fix works as intended:

  1. Run the optimisation script - "Supply system part II (centralised)" with the 'Advanced' setting "generate-detailed-outputs" turned on.
  2. Have a look at the operational performance profiles under 'SCENARIO\outputs\data\optimization\centralized\DCS_XXX\Supply_system_operation_details' e.g. 'N1001_operation.csv'.

If the profiles appear in that file, that means the fix worked.

reyery commented 4 months ago

https://github.com/architecture-building-systems/CityEnergyAnalyst/blob/acbdbef1fa33652a8ccc6a3750d0303d3b540405/cea/optimization_new/domain.py#L471-L500

@MatNif is there a reason why you set the index as numerical indexes when summing them, then adding the date index back again when combining the profiles? I assume that the datetime index should be the same for all the different profiles, so summing them directly, should not cause any issues.

MatNif commented 4 months ago

@reyery, no, you're right. You're referring to the else statements, right? There's no need for the indexes to be numerical at first.

reyery commented 4 months ago

Yeah the else statements, but even for the if statements when doing the concat. You are using .values() when doing sum, instead of the original series which I assume includes the date index.

I guess the only advantage I see for using .values() is that you would be able to concat/sum the values together without any errors if the indexes are not the same. But the profiles should have the same date index anyway, unless something went wrong somewhere. So I think it is better to use the whole series and if the length of the series is not expected after concat/sum (i.e. not equal to the number of rows of the date time index), then there is an error somewhere.

MatNif commented 4 months ago

Hi @reyery,

I've adapted the parts in the else statements as you suggested.

The parts in the if statements actually already retain the date-time indexes. All three objects (supply_system.heat_rejection, supply_system.greenhouse_gas_emissions and supply_system.system_energy_demand) are dictionaries which contain energy_carrier_codes as keys and pandas Series as values. So what I could write alternatively would be the following:

combined_heat_rejection_profile = pd.concat([supply_system.heat_rejection[energy_carrier] for energy_carrier in supply_system.heat_rejection.keys()], axis=1).sum(1)

I'm not sure how it compares to the current version in terms of performance, but I feel like the current version at least looks a bit cleaner. Let me know what you think.

ShiZhongming commented 3 months ago

Thanks for the PR. @MatNif I have tested it using both SG and CH cases. Both working fine.

ShiZhongming commented 3 months ago

After the master was merged into this PR, all SG, CH and DE cases all break at demand. However, the optimisation script of this PR works fine. W