Skip to content

Conversation

@mjprilliman
Copy link
Collaborator

-Added LCOS calculation to financial models
-To be used for models with battery, potentially CSP technologies
-Current changes causing random crashes in UI, haven't yet traced it to particular change


// Change the timestep of the battery and its component models
void ChangeTimestep(double dt_hr);

Copy link
Collaborator

Choose a reason for hiding this comment

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

So this might be a merge conflict, as this shouldn't be getting deleted. I'm noticing some other changes spread throughout such as dt_hr being changed back to dt_hour in battery state structs. And in cmod_stateful as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure how I managed to do that but thanks for the catch. Fixed those potential merge conflicts and pushed again.

@dguittet
Copy link
Collaborator

dguittet commented Feb 8, 2021

@mjprilliman Exciting! Do you mind adding some references for LCOS and its calculation here? Also, if there's a place to make sense to add that reference as a comment in the code, that would be really helpful for future maintenance as well.

@mjprilliman
Copy link
Collaborator Author

@mjprilliman Exciting! Do you mind adding some references for LCOS and its calculation here? Also, if there's a place to make sense to add that reference as a comment in the code, that would be really helpful for future maintenance as well.

https://www.cell.com/joule/pdfExtended/S2542-4351(18)30583-X

That is the main reference we have been going off of. I will be sure to include references in the code as we go.

@mjprilliman mjprilliman requested a review from cpaulgilman May 3, 2021 19:56
Copy link
Collaborator

@cpaulgilman cpaulgilman left a comment

Choose a reason for hiding this comment

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

Should the variable nameplate1 be named something like nameplate_battery or battery_nameplate for clarity?

int grid_charging_cost_version = 0;
lcos_calc(this, cf_lcos, nyears, nom_discount_rate, inflation_rate, lcoe_real, cost_prefinancing, disc_real, grid_charging_cost_version, 0);
/*
double lcos_investment_cost = as_double("battery_total_cost_lcos"); //does not include replacement costs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this commented out code be removed now that lcos calc is in a function?


ssc_number_t tier_credit = 0.0, sr = 0.0, tier_energy = 0.0;
// time step sell rates
if (as_boolean("ur_en_ts_sell_rate")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should always be false. ur_calc only runs for metering options 0 and 1, and the GUI disables time series rates for those options.

}

// Fall back to TOU rates if m_ec_ts_sell_rate.size() is too small
if (tier_credit == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that the above is expected to always be false - this will always be true.


}
}
if (as_boolean("ur_en_ts_buy_rate")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as sell rates: I'd expect this is always false.


ssc_number_t tier_charge = 0.0, br = 0.0, tier_energy = 0.0;
// time step sell rates
if (as_boolean("ur_en_ts_buy_rate")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as sell rates: I'd expect this is always false.

monthly_elec_needed_from_grid_batt(12),
monthly_cumulative_excess_energy_batt(12), monthly_cumulative_excess_dollars_batt(12), monthly_bill_batt(12), monthly_peak_batt(12), monthly_test_batt(12),
monthly_two_meter_sales_batt(12),
monthly_true_up_credits_batt(12); // Realistically only one true-up month will be non-zero, but track them all for monthly outputs
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused by these new inputs. It looks like they aren't actually used?

Additionally a lot of the computations inside ur_calc don't seem to make it outside of that function.

It looks like the only thing that is happening here is the computation of e_batt_cy and p_batt_cy, is the rest of the code needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally I thought there would need to be changes to utility rate code to calculate the grid charging costs. After conversations I ended up not needing much or any of these code changes. I tried to remove changes but I see I missed a lot. I'm looking through it but I'm fairly confident this can be reverted to develop

@mjprilliman mjprilliman merged commit f4e04b5 into develop Jul 2, 2021
@mjprilliman mjprilliman deleted the lcos_cost_inputs branch July 2, 2021 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants