REopt_API icon indicating copy to clipboard operation
REopt_API copied to clipboard

Add CoolingLoads model to job app

Open rathod-b opened this issue 3 years ago • 8 comments

Also update the manifest to reflect latest REopt.jl version

Please check if the PR fulfills these requirements

  • [x] CHANGELOG.md is updated
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [x] Docs have been added / updated (for bug fixes / features)

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.

rathod-b avatar Jun 29 '22 21:06 rathod-b

@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?

Bill-Becker avatar Jul 13 '22 14:07 Bill-Becker

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 avatar Jul 13 '22 16:07 rathod-b

@rathod-b I think we discussed adding ExistingChiller in this same PR, right? I think that makes sense because cooling load by itself doesn't really add much functionality without the BAU way to serve that cooling load. I think that's what I intended when I made the "add cooling load to API /job app" issue (I don't see another issue for adding ExistingChiller), and I just updated the title of that issue to make that explicit. Then we'll have some useful outputs for the ExistingChiller too.

Bill-Becker avatar Aug 01 '22 21:08 Bill-Becker

@Bill-Becker with this branch now caught up with develop, the PR is ready for your second review.

zolanaj avatar Sep 12 '22 20:09 zolanaj

@rathod-b @zolanaj I'm realizing that we need to add CoolingLoad in the results in REopt.jl and then add CoolingLoadOutputs model in the API to get the processed load inputs like the hourly loads (from a CRB). The UI in particular needs this for charting how the load is served. I was thinking we could assign some processed inputs to the API models by creating the scenario and/or reopt_inputs structs before run_reopt. Looks like we are doing something to that effect here. Thoughts on doing that versus just keeping all/most model field assignment from REopt.jl -> REopt_API models in the results/outputs from REopt.jl?

Bill-Becker avatar Sep 15 '22 14:09 Bill-Becker

@Bill-Becker I'm inclined to mirror what's already done with the electric load as I think that would be easier: https://github.com/NREL/REopt.jl/blob/master/src/results/electric_load.jl

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 CoolingLoad. Does that sound good? I can turn this around pretty quickly (after addressing the PR backlog).

Do we need this for the heating load in results in REopt.jl as well? I didn't see it anywhere.

zolanaj avatar Sep 15 '22 16:09 zolanaj

@zolanaj awesome, thanks! And yes, we'd need this for the heating load(S) as well.

Bill-Becker avatar Sep 15 '22 18:09 Bill-Becker

@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 avatar Oct 24 '22 18:10 Bill-Becker

@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.

zolanaj avatar Nov 11 '22 23:11 zolanaj

@zolanaj Can you see why we don't get CoolingLoad in the outputs of the response? I added a test (temporary) in 8cbe1bfcefac5a38e25bfab84ac0a595a9ff7c07 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 avatar Nov 16 '22 05:11 Bill-Becker

@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?

zolanaj avatar Dec 01 '22 19:12 zolanaj

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):

output keys: dict_keys(['Financial', 'ElectricTariff', 'ElectricUtility', 'ElectricLoad', 'Site', 'PV']) inputs keys: dict_keys(['Financial', 'ElectricLoad', 'Site', 'Settings', 'PV', 'ElectricTariff', 'ElectricUtility', 'CoolingLoad', 'ExistingChiller', 'ExistingBoiler', 'SpaceHeatingLoad', 'DomesticHotWaterLoad', 'CHP'])

So the CHP seems to be missing from the outputs as well. I'll keep digging into this.

zolanaj avatar Dec 02 '22 05:12 zolanaj

@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?

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 avatar Dec 02 '22 16:12 Bill-Becker

@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.)

zolanaj avatar Dec 09 '22 18:12 zolanaj