Sector dependent defaults#662
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements sector-dependent defaults for the REopt API, allowing different financial and tax parameters based on the user's sector (commercial/industrial or federal) and additional federal procurement details.
Key changes:
- Added sector-specific default values that vary based on sector type and federal procurement parameters
- Introduced new sector-related input fields to the Site model
- Modified existing default values to be sector-dependent rather than hardcoded
Reviewed Changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if post["CHP"].get(key) is None: # Check that default got assigned consistent with /chp_defaults | ||
| if key == "max_kw": | ||
| self.assertEquals(inputs_chp[key], view_response["chp_max_size_kw"]) | ||
| print(r.get("messages")) |
There was a problem hiding this comment.
Debug print statements should be removed from production test code. These print statements appear to be left over from development and debugging.
| print(r.get("messages")) |
There was a problem hiding this comment.
will rm when I finish updates
| print(model_name) | ||
| print(input_key) |
There was a problem hiding this comment.
Debug print statements should be removed from production test code. These print statements appear to be left over from development and debugging.
| open("debug_reopt_inputs_to_julia.json","w") do f | ||
| JSON.print(f, d, 4) | ||
| end |
There was a problem hiding this comment.
Debug file output statements should be removed from production code. These statements write debug JSON files during every optimization run, which could impact performance and disk usage in production.
| open("debug_reopt_inputs_to_julia.json","w") do f | |
| JSON.print(f, d, 4) | |
| end |
There was a problem hiding this comment.
Hallie are you planning to remove these lines (or comment them out)?
There was a problem hiding this comment.
yes I will rm, just waiting cause I might need them when I point to the updated julia package
| open("debug_reopt_all_inputs_from_julia.json","w") do f | ||
| JSON.print(f, struct_to_dict(model_inputs.s), 4) | ||
| end | ||
| open("debug_reopt_inputs_with_defaults_from_julia.json","w") do f | ||
| JSON.print(f, inputs_with_defaults_set_in_julia, 4) | ||
| end |
There was a problem hiding this comment.
Debug file output statements should be removed from production code. These statements write debug JSON files during every optimization run, which could impact performance and disk usage in production.
| open("debug_reopt_all_inputs_from_julia.json","w") do f | |
| JSON.print(f, struct_to_dict(model_inputs.s), 4) | |
| end | |
| open("debug_reopt_inputs_with_defaults_from_julia.json","w") do f | |
| JSON.print(f, inputs_with_defaults_set_in_julia, 4) | |
| end |
| blank=True, | ||
| null=False, | ||
| default=SECTORS.COMMERCIAL, | ||
| help_text=("") |
There was a problem hiding this comment.
Note to merge develop and add "HighTempThermalStorage" here and anywhere else that's needed
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
b/c my julia is suddenly having permissions issues and I can't run anything
…into federal-defaults
added federal_itc_fraction to HighTempThermalStorage struct
Still TODO, some can probably wait til after FY end:
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
feature
What is the current behavior?
1 set of defaults based on commercial/industrial analyses
What is the new behavior (if this is a feature change)?
defaults change depending on user specified sector (and additional details if sector=federal)
Does this PR introduce a breaking change?
no
Other information: