Skip to content

Batt functions for reopt sizing#1022

Merged
brtietz merged 9 commits into
patchfrom
batt_functions_for_reopt_sizing
May 10, 2023
Merged

Batt functions for reopt sizing#1022
brtietz merged 9 commits into
patchfrom
batt_functions_for_reopt_sizing

Conversation

@brtietz

@brtietz brtietz commented May 9, 2023

Copy link
Copy Markdown
Collaborator

Add new function Reopt_size_standalone_battery_params to assist with dGen PV and battery sizing using the battery compute module. Also update URDB Python call to version 8.

Pairs with SAM PR NatLabRockies/SAM#1398 and PySAM PR NatLabRockies/pysam#149

Comment thread ssc/cmod_battery_eqns.cpp
return true;
}

bool Reopt_size_standalone_battery_params(ssc_data_t data) {

@dguittet dguittet May 9, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you make a test for this? Perhaps similar to TEST_F(CMPvsamv1PowerIntegration_cmod_pvsamv1, reopt_sizing)

Comment thread ssc/cmod_battery_eqns.cpp

// financial inputs
map_optional_input(vt, "itc_fed_percent", &reopt_batt, "total_itc_pct", 0., true);
// TODO: what about reopt vars total_rebate_us_dollars_per_kw?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What about this TODO? Are there any incentives?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That TODO has been in there for a while and got copy-pasted over. I'm thinking this TODO might be a better fit for a larger REopt API version upgrade closer to release time, the latest version doesn't include that variable: https://nrel.github.io/REopt.jl/dev/reopt/inputs/#Financial

Comment thread ssc/cmod_battery_eqns.h Outdated
"Conditional: only required if the variable it's meant to replace is missing.\\n"
"REopt's 'microgrid_upgrade_cost_pct' set to 0 because SAM doesn't apply this cost multiplier.\\n"
"REopt inputs not used in this function will be REopt's defaults.\\n"
"Some additional REopt inputs are required, such as lon and lat. See https://nrel.github.io/REopt.jl/dev/reopt/inputs/ for a full set of inputs\\n\\n"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is lat and lon still required?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. Though the intended use of this function will include prod_factor_kw for any PV, those are required inputs in the REopt API: https://nrel.github.io/REopt.jl/dev/reopt/inputs/#Site

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see lat and lon in the new battery eqn being passed in?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The battery compute module does not have lat and lon, they need to be added to the post after this function call.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see that it's added in the pv equation. So is this standalone battery post a viable submission to REopt in itself?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is not. This line is intended to be a hint that additional inputs are required, is there a better way to phrase it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"The post produced here is not a complete submission. Further additions are required, such as x and y, performed by function z"?

In that case, what is the intended use case in adding it to PySAM's standalone battery module? The user then adds the missing variables?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, better explanation in latest push.

Yes, for the planned dGen use case Python code will add the additional required variables. dGen uses the Battery compute module, but not the PV compute module, so the existing function was not an option.

@dguittet dguittet self-requested a review May 10, 2023 16:32
@brtietz brtietz merged commit 9618862 into patch May 10, 2023
@brtietz brtietz deleted the batt_functions_for_reopt_sizing branch December 13, 2023 16:23
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.

3 participants