Add CoolingLoads model to job app #329
Conversation
Also update the manifest to reflect latest REopt.jl version
|
@rathod-b I was working on adding GHP to the /job endpoint and realized we didn't have cooling load in there yet. Are you actively working on this? Do you need any help? |
|
I will be able to actively work on this once we merge off-grid into develop. There is a dependency on the off-grid PR because it has a lot of input/output field name changes which are in REopt, but not in the API#develop at the moment. |
|
@rathod-b I think we discussed adding |
Added existing chiller inputs and outputs, updated TOML files to use REopt v0.17.0, updated migration file
Add existingchiller updates
|
@Bill-Becker with this branch now caught up with develop, the PR is ready for your second review. |
|
@rathod-b @zolanaj I'm realizing that we need to add |
|
@Bill-Becker I'm inclined to mirror what's already done with the electric load as I think that would be easier: I can replicate these for hot and cold thermal loads and add to REopt.jl first, then add to this branch PR to import the results for Do we need this for the heating load in results in REopt.jl as well? I didn't see it anywhere. |
|
@zolanaj awesome, thanks! And yes, we'd need this for the heating load(S) as well. |
|
@zolanaj The cooling and heating load outputs that you added in REopt.jl are now merged in the published version, so we can continue to add them here. |
|
@Bill-Becker I think this is ready for another pass by you. I am merging this branch into add-hot-tes, where I also added cold thermal storage handling. To keep scope manageable, I think it's best to leave as two separate PRs for now. |
May be temporary just to debug CoolingLoad inputs and outputs
|
@zolanaj Can you see why we don't get CoolingLoad in the outputs of the response? I added a test (temporary) in 8cbe1bf in test_job_endpoint that just writes out the response to a .json in your top-level directory, and I'd expect to see CoolingLoad in the outputs. In REopt.jl, the same set of inputs gives me the CoolingLoad key in the results, so I'm not sure why it's not passing that through in process_results. |
|
@Bill-Becker after addressing a couple different nits, it looks like the underlying issue is that you can add a cooling load or a heating load without any technologies to serve said loads. If you do this, the Julia model simply doesn't generate the cooling or heating constraints (which check for technology sets rather than loads) and you won't get the outputs for the thermal loads or anything else. I'll suggest adding an issue to REopt.jl to handle loads without any technologies to serve them by throwing an error instead of infeasible (I'll add it and assign myself as I think I know where to put it), and also add some checks on the API side so it returns a warning and automatically adds an existing default chiller or boiler as needed. Does that path sound good to you? |
|
Actually there's more to it than that. Here's a summary of the keys to the inputs and outputs of the response dictionary with a post that includes CHP and heating and cooling loads (taken from REopt.jl's posts for heating and cooling load inputs):
So the CHP seems to be missing from the outputs as well. I'll keep digging into this. |
In scenario.jl, it creates those "BAU" cooling and heating techs if there is a cooling and heating load, so those constraints should be getting activated without any chiller or boiler inputs. I used the same set of inputs in REopt.jl with only CoolingLoad, no techs, and I'm get CoolingLoad in the results. |
|
@Bill-Becker this is ready for your next round of review. The problem was a mismatch in the HeatingLoad and ExistingChiller outputs, which, once found in views.py here, rather than return an error the process exits gracefully with the outputs generated up to that point. There was nothing wrong with CHP outputs - it just never got to that code. Something to be aware of when generating new outputs objects. I made some updates to the temporary test but am thinking it's good to keep in for this and future outputs testing. I removed BAU and it sped things up quite a bit. What do you think? (I can make a separate post .json instead of a hard-coded dictionary if you prefer.) |
| if len(self.blended_doe_reference_names) > 0 and self.doe_reference_name == "": | ||
| if len(self.blended_doe_reference_names) != len(self.blended_doe_reference_percents): | ||
| error_messages["blended_doe_reference_names"] = \ | ||
| "The number of blended_doe_reference_names must equal the number of blended_doe_reference_percents." | ||
| if not math.isclose(sum(self.blended_doe_reference_percents), 1.0): | ||
| error_messages["blended_doe_reference_percents"] = "Sum must = 1.0." | ||
|
|
||
| if self.doe_reference_name != "" or \ | ||
| len(self.blended_doe_reference_names) > 0: | ||
| self.year = 2017 # the validator provides an "info" message regarding this) | ||
|
|
||
| if len(self.monthly_fractions_of_electric_load) > 0: | ||
| if len(self.monthly_fractions_of_electric_load) != 12: | ||
| error_messages["monthly_fractions_of_electric_load"] = \ | ||
| "Provided cooling monthly_fractions_of_electric_load array does not have 12 values." | ||
|
|
||
| # Require 12 values if monthly_tonhours is provided. | ||
| if 12 > len(self.monthly_tonhour) > 0: | ||
| error_messages["required inputs"] = \ | ||
| "Must provide 12 elements as inputs to monthly_tonhour. Received {}.".format(self.monthly_tonhour) |
There was a problem hiding this comment.
@zolanaj Thoughts on needing this validation if it's also done in Julia?
There was a problem hiding this comment.
(the same comment would apply to the current validation in electric and heating loads) I believe all of these validations were before we could pass back the errors from Julia. I'm now thinking that when we merge the in-process "error passing from Julia" PRs, we do a scrub of unnecessary/duplicate validation. So no action needed!
There was a problem hiding this comment.
Thanks @Bill-Becker! I added issue #387 to note this as a future development effort and will merge this now.
Also update the manifest to reflect latest REopt.jl version
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
(Bug fix, feature, docs update, ...)
Feature
REopt v3 API users can now include CoolingLoad and ExistingChiller keys to their input JSONs and get ExistingChiller key returned in results.
What is the current behavior?
(You can also link to an open issue here)
Per #303, at present REopt v3 API users can't include CoolingLoads or ExistingChiller in their input JSON even though REopt.jl supports these capabilities.
What is the new behavior (if this is a feature change)?
Now, REopt v3 API users can't include CoolingLoads or ExistingChiller in their input JSON to utilize these capabilities in REopt.jl.
Does this PR introduce a breaking change?
(What changes might users need to make in their application due to this PR?)
No, these keys are optional and will not break existing functionality for inputs or outputs.
Other information:
There was no validation happening for these fields except for a few time series validations for cooling loads, so no tests have been added yet. Ideally we can add a comprehensive validation test once other thermal heating/cooling capabilities have been added.