Conversation
…ed off of feature/parametrize_charges) In utils: - Updating sum() util function to use pyo.summation - Adding decompose_consumption function to split a np array, pyo variable, or cvxpy variable into positive and negative components - Adding initialize_decomposed_pyo_vars to help with initializing the decomposed elements in a pyomo model Modifying existing tests in test_costs.py to include decompositino Adding test_decompose_consumption_np, test_decompose_consumption_cvxpy, test_decompose_consumption_pyo to test_utils.py Adding documentation notes in how_to_advanced Adding test coverage for negative values input to energy / demand calculation Removing ValueError raised in calculate_cost for non-np.array/pyo/cvx object. Since this code is never actually reached because the valueerror will be raised from ut.multiply() first Consolidating consumption_data_dict and consumption_object_dict arguments Adding magnitude constraint so that positive_var[t] + negative_var[t] == abs(expression[t]) to prevent both variables becoming large when export rate is higher than energy rate Reformatted with black Updated test_calculate_cost_pyo to use ipopt rather than gurobi when running decompose_exports, since abs() function is nonlinear / nonconvex
…ct and parametrize_rate_data Adding test_calculate_itemized_costs for np, cvx, and pyo with one non-decomposed and one decomposed example Adding a warning test to calculate_export_revenue Adding helper functions for pyo and cvx problems to be used by their versions of both test_calculate_cost and test_calculate_itemized_cost Adding warning that decompose_exports isn't supported with cvxpy due to non-DCP exports-imports issues Reformatted with black
…take None, "absolute_value", "binary", or other types in the future Using max_pos for creating positive and negative pyomo variables (WIP: Index issue failing test in test_utils)
Adding calculate_conversion_factors Modifying calculate_energy_cost to only apply convex approximation when there are charge tiers TESTS Adding consumption_estimate to test_calculate_cost_cvx to avoid default 0 issues Adding additional test case for cvx problem without charge limit to test_calculate_cost_cvx Updating test expectations in test_max_pos_pyo so that scalar inputs have scalar expectations, vector inputs have vector expectations
Reformatting with black
…position, removing it from divisor in costs.py and renaming divisor to n_per_hour Moving application of conversion factor to helper function get_converted_consumption_data
default_varstr_alias_func takes row index parameter, where index is used if "name" is blank Removing dashes from names in test_costs.py since sanitize_varstr removes dashes and replaces with underscores Cleaning up error expectation in test_calculate_cost_np
Adding back get_unique_row_name (renamed to get_charge_name) Removing initialize_decomposed_pyo_vars. The variables self-initialize during solve. Updating test_decompose_consumption_pyo to just check for the presence of objects, not values. (WIP to add value check in test_calculate_cost_pyo for pre-decomposed inputs) Renaming _get_decomposed_var_names to get_decomposed_var_names Adding test cases for extended consumption data format in test_calculate_cost_np and test_calculate_cost_pyo
Updating minimum pyomo version to 6.8
- Add scalar LinearExpression tests for max_pos_pyo - Add TypeError tests for invalid types in max_pos and decompose_consumption - Add warning tests for unimplemented decomposition types - Add ValueError test for invalid type in calculate_export_revenue - Add negative values warning test in calculate_demand_costs - Add test for consumption_estimate=None and dict consumption_estimate - Add conversion factor test with MW units in test_calculate_itemized_cost_pyo - Fix idempotency in get_converted_consumption_data to prevent duplicate components - Add warnings for unimplemented decomposition_type in utils.decompose_consumption
…composition has already been run on the model
|
Closing the old PR (#33) and opening a new PR for rebased branch. Prior notes from @dalyw Some pseudocode to explain the approach more:
In Pyomo, this is implemented as:
In Pyomo, this is implemented as:
In Pyomo, we add an additional constraint to prevent “cheating” when export price is higher than energy rates and the solver tries to increase imports to infinity
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 91.00% 93.40% +2.40%
==========================================
Files 10 10
Lines 1356 1593 +237
==========================================
+ Hits 1234 1488 +254
+ Misses 122 105 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return f"{utility}_positive", f"{utility}_negative" | ||
|
|
||
|
|
||
| def decompose_consumption( |
There was a problem hiding this comment.
If we have a few types of decomposition, I could see how this could be a bit tough to follow. You might consider having a function for each decomposition type (i.e. decompose_absolute_value, decompose_binary, etc.) that are called within decompose_consumption.
There was a problem hiding this comment.
That's a good idea to make it more readable. I might work on the binary version so that it will work with gurobi in the next couple of weeks and will separate them
| .. code-block:: python | ||
|
|
||
| from eeco import costs | ||
| from electric_emission_cost import costs |
There was a problem hiding this comment.
Is this an older version using electric_emission_cost name rather than eeco?
| Options: | ||
| - Default `None` | ||
| - `"binary_variable"`: To be implemented | ||
| - `"absolute_value"` |
There was a problem hiding this comment.
This formatting isn't working, I think due to indentation?
Should be
Options:
- Default
None "absolute_value""binary_variable": To be implemented
There was a problem hiding this comment.
Fixed! There needs to be a blank line before and after the enumerated list
| new_start.strftime("%Y%m%d"), | ||
| str(limit), | ||
| ) | ||
| key_str = default_varstr_alias_func( |
There was a problem hiding this comment.
Good use of this for consistency!
| "Please switch to new 'charge (metric)' and 'charge (imperial)' format", | ||
| "Please switch to new 'charge (metric)' and 'charge (imperial)' format. " | ||
| "Current behavior assumes units of therms (imperial) " | ||
| "and converts to cubic meters (metric) when unspecified.", |
There was a problem hiding this comment.
Not necessary here, but could consider if an optional electric_charge_units or gas_charge_units argument could be useful to handle cases where the charge data is not in standard units. (But that would only be if someone brought in their own billing data)
There was a problem hiding this comment.
Yeah I guess we could do that, but if you bring your own data you can just convert to one of those two units is my thought. My plan is to actually remove the support for the old format entirely after iterating a couple more versions. it's mostly this way to support the old billing files that Pepe used (hence the DeprecationWarning).
I would just have a single column except for that the raw data is almost always in therms in the US, but scientists usually use metric, so I thought providing that conversion would be nice.
For the majority of users who just calculate electricity bills and not natural gas the imperial and metric columns are the same anyway.
|
Looks good (assuming the linting failure gets resolved!) Just two minor changes on the how_to_advanced.rst file |
Pull request recommendations:
In
utils:In
costs:Fixes to
parametrize_charge_dict:Updating
scale_ratiosargument name topercent_change_dictRemoving
get_unique_row_nameand consolidating withdefault_varstr_alias_funcProvide relevant tests for your feature or bug fix.
Provide or update documentation for any feature added by your pull request.
Thanks for contributing!