Bess size class - #732
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new endpoint currently fails to propagate upstream HTTP status codes and there are additional correctness/contract gaps (size_class docs/validation mismatch, missing test coverage and test fixture updates, and a risky Julia dependency pin).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds support for an ElectricStorage (BESS) size_class input and a new cost-defaults endpoint that proxies from the Django API to the Julia HTTP service, enabling battery cost defaults to be derived from size class / load metrics.
Changes:
- Added
/electric_storage_cost_defaultsDjango endpoint + URL route to proxy to Julia. - Introduced
ElectricStorageInputs.size_classand made ElectricStorage installed cost fields nullable (so defaults can be computed upstream). - Added a Julia
/electric_storage_cost_defaultsendpoint and updated the Julia environment to a REopt.jl revision that supports the new field.
File summaries
| File | Description |
|---|---|
| reoptjl/views.py | Adds electric_storage_cost_defaults Django view that calls the Julia service. |
| reoptjl/urls.py | Registers the new Django route for the cost-defaults endpoint. |
| reoptjl/models.py | Adds size_class to ElectricStorageInputs and makes cost fields nullable. |
| reoptjl/migrations/0126_electricstorageinputs_size_class_and_more.py | Database migration for the new field + altered cost fields. |
| julia_src/Manifest.toml | Pins REopt.jl dependency to a specific git source/revision metadata. |
| julia_src/http.jl | Adds Julia HTTP handler for electric_storage_cost_defaults and returns size_class defaults. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| git-tree-sha1 = "cbd0675c03f309a4a8bf4ff8048783d2c9c59f79" | ||
| repo-rev = "electric_storage_size_class" | ||
| repo-url = "https://github.com/NatLabRockies/REopt.jl.git" |
There was a problem hiding this comment.
Will be fixed before merging into master.
| migrations.AddField( | ||
| model_name='electricstorageinputs', | ||
| name='size_class', | ||
| field=models.IntegerField(blank=True, help_text='ElectricStorage size class. Must be an integer value between 1 and 3. Default is calculated per ratio of annual peak and average load of given load profile.', null=True, validators=[django.core.validators.MinValueValidator(1), django.core.validators.MaxValueValidator(5)]), | ||
| ), |
There was a problem hiding this comment.
We do not want to add this standalone for electric storage. best to leave out size class or add for all techs together which could cause infeasibilities with existing scenario.
| response = JsonResponse( | ||
| http_jl_response.json() | ||
| ) | ||
| return response |
| exc_type, exc_value, exc_traceback = sys.exc_info() | ||
| debug_msg = "exc_type: {}; exc_value: {}; exc_traceback: {}".format(exc_type, exc_value.args[0], | ||
| tb.format_tb(exc_traceback)) |
| MaxValueValidator(5) | ||
| ], | ||
| null=True, | ||
| blank=True, | ||
| help_text="ElectricStorage size class. Must be an integer value between 1 and 3. Default is calculated per ratio of annual peak and average load of given load profile." |
| def electric_storage_cost_defaults(request): | ||
|
|
||
| if request.method == "POST": | ||
| inputs = json.loads(request.body) | ||
| else: |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
(Bug fix, feature, docs update, ...)
What is the current behavior?
(You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?
(What changes might users need to make in their application due to this PR?)
Other information: