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
| File | Description |
|---|---|
| reoptjl/views.py | Added new sector_defaults view function to handle sector-specific default requests |
| reoptjl/urls.py | Added URL routing for the new sector_defaults endpoint |
| reoptjl/models.py | Added sector fields to SiteInputs and removed hardcoded defaults from financial/tech models |
| reoptjl/migrations/ | Database migration files for the new sector fields and nullable default values |
| julia_src/http.jl | Added sector_defaults endpoint and expanded inputs_with_defaults_set_in_julia tracking |
| reoptjl/test/ | Updated test files with sector defaults testing and commented out other tests |
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 |
reoptjl/models.py
Outdated
| blank=True, | ||
| null=False, | ||
| default=SECTORS.COMMERCIAL, | ||
| help_text=("") |
julia_src/http.jl
Outdated
| hot_storage_dict = Dict(key=>getfield(model_inputs.s.storage.attr["HotThermalStorage"], key) for key in inputs_with_defaults_from_julia_hot_storage) | ||
| else | ||
| hot_storage_dict = Dict() | ||
| end |
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: