Update the HVAC Model - Githubissues

hlngo commented 5 years ago

@ORNLJD There are couple of items I would like to clarify:

It seems like you are using the old "fleet_interface.py"? Can you copy the "src/fleet_interface.py" from this repository to your local machine, update your code accordingly, and make another PR.
Why do you need 2 files "src/fleets/HVAC_fleet/fleet_request.py" and "src/fleets/HVAC_fleet/grid_info.py"? You should have referred to them using the Python:
```
from os.path import dirname, abspath, join
sys.path.insert(0,dirname(dirname(dirname(abspath(__file__)))))
from fleet_request import FleetRequest
from fleet_interface import FleetInterface
```
Example: https://github.com/GMLC-1-4-2/battery_interface/blob/a0c8a05296a6219e2884ba807e72d0c8dde7afa2/src/fleets/battery_inverter_fleet/battery_inverter_fleet.py#L6

ORNLJD commented 5 years ago

Thank you Hung. I will check out the latest 'fleet_interface' file. Previously, I had issues calling those 2 files, I tired test by copying them to my local fleet folder. I will fix these, and also add the Readme file.

hlngo commented 5 years ago

@ORNLJD Thanks. That helps a lot.

ORNLJD commented 5 years ago

Revised the HVAC fleet by adding README, and tested with the regulation services. Has a conflict with the test.py file. So created a separate test_jin file.

ORNLJD commented 5 years ago

Add frequency-watt response for both HVAC and Refrigeration models. Add output_impact metrics for both HVAC and Refrigeration models. Run through the unit tests for both models (skip the round-trip efficiencies).

Hayden-Reeve commented 5 years ago

@ORNLJD (cc: @hlngo , @emayhorn )

Jin - I have started to run the HVAC code and run into the following issues:

You have a lot of duplicate versions of utility files in your HVAC folder. We have asked you to remove these in the past. Please delete them and use the versions in the \src folder. These include: HVAC\grid_info HVAC\fleet_interface HVAC\fleet_response HVAC\fleet_request Please also remove test_jin and test_unit2 from your commit. We should also remove the entire home_ac folder from the repository as it is obsolete.
When running for Reserve service the code crashed at the lines below as the datetime format did not match the format specified. The purpose of these lines is not clear to me. They seem to take a datetime parameter, turn it into a string, then turn it back into a datetime parameter. I got the code to run by using dt=ds and dt=start_time. I am not sure this is a robust fix as reserve service ran but no P_service values were shown in the plots.

https://github.com/GMLC-1-4-2/battery_interface/blob/18cc20c27c7812b7ff1d274f56e29ce81668c948/src/fleets/HVAC_fleet/HVAC_fleet.py#L938

https://github.com/GMLC-1-4-2/battery_interface/blob/18cc20c27c7812b7ff1d274f56e29ce81668c948/src/fleets/HVAC_fleet/HVAC_fleet.py#L1008

When running artificial inertia the code crashed as an array is provided for each input to Frequency_watt and get_frequency. The frequency evaluation then crashes: https://github.com/GMLC-1-4-2/battery_interface/blob/18cc20c27c7812b7ff1d274f56e29ce81668c948/src/fleets/HVAC_fleet/HVAC_fleet.py#L421

Recommend you follow the EV example and pass each subfleet in individually: https://github.com/GMLC-1-4-2/battery_interface/blob/f42b3d3115e4ed0e7920e0df2e0e6ea0edd3b090/src/fleets/electric_vehicles_fleet/electric_vehicles_fleet.py#L475-L486

When running Regulation Service it crashes at the timestamp 2017-08-01 17:36:00 even when I change time_steps due to IndexError: index 2880 is out of bounds for axis 0 with size 2880 at: https://github.com/GMLC-1-4-2/battery_interface/blob/18cc20c27c7812b7ff1d274f56e29ce81668c948/src/fleets/HVAC_fleet/HVAC_fleet.py#L548-L552

I am not sure why this is - please investigate.

Hayden-Reeve commented 5 years ago

@ORNLJD (CC: @hlngo @emayhorn )

I have noticed something else strange with HVAC. When I run with both Reserve and Voltage Regulation the FleetResponse from Process_Request overrides all elements of responses when appended (not just adding to the last one). This results in the entire response list having the same timestamp (ts) and P_service etc. This does not happen with other devices (I tried with PV and it worked as expected). I would recommend that you run your code with Reserve in debug mode with stops at the append line to see this in action. I am not smart enough to understand why it is doing this. I would recommend you investigate this (and the issues in the post above) and resubmit.

https://github.com/GMLC-1-4-2/battery_interface/blob/f42b3d3115e4ed0e7920e0df2e0e6ea0edd3b090/src/services/distribution_voltage_regulation/distribution_regulation_service.py#L125

https://github.com/GMLC-1-4-2/battery_interface/blob/f42b3d3115e4ed0e7920e0df2e0e6ea0edd3b090/src/services/reserve_service/reserve_service.py#L293

hlngo commented 5 years ago

@ORNLJD Please address Hayden's and my reviews above. @Hayden-Reeve It is because the HVAC process_request code refers to the same instance of FleetResponse for all requests, thus the last response overrides all fields of previous responses, which is wrong.

hlngo commented 5 years ago

@Hayden-Reeve Do you want to take another look at this PR?

Hayden-Reeve commented 5 years ago

Yes. @ORNLJD is it ready for re-testing? It does not look like the other issues have been addressed yet.

ORNLJD commented 5 years ago

@Hayden-Reeve, @hlngo Thanks for pointing out those issues. I have resolved most of the problems, but still investigating the override issue. I will submit another pull request once I debugged that.

ORNLJD commented 5 years ago

@Hayden-Reeve (CC: @hlngo @emayhorn ) Hi Hayden, please do a retest for the HVAC code.

Hayden-Reeve commented 5 years ago

@ORNLJD (cc: @hlngo )

Jin, I ran a clean clone version of the library with and without PR #77 . I was not able to reproduce the issue you are having with the timestep/stamp bouncing around the place and not monotonically increasing (as time should). I put some examples below of your and my results for regulation. I recommend that you create a second instance of just the clean clone of the library and see if the issue persists. If so, it may be a package issue or some other environment issue. If this does not fix it @hlngo may have some ideas.

Hayden Result Dynamic Regulation: 20190506_August_2secnormsignals_Dynamic_HVACFleet

Jin Result Dynamic Regulation: 20190504_August_2secnormsignals_Dynamic_HVACFleet

Also, I ran reserve service and it did not crash - so I was not able to reproduce your issue there. Maybe that will resolve itself once you check out a clean version:

20190506_annual_signals_HVACFleet

Finally, the code did crash for peak management service. I need to investigate further and will post separately on this.

Hayden-Reeve commented 5 years ago

@ORNLJD (cc: @emayhorn )

Peak Management Service is failing with HVAC at the following command because IndexError: index 192 is out of bounds for axis 0 with size 192:

https://github.com/GMLC-1-4-2/battery_interface/blob/e8244c1a70704145ac2aca5b2d968713a79a1b1e/src/fleets/HVAC_fleet/HVAC_fleet.py#L539-L543

I have seen this index go out of bounds before too. This looks too detailed for me to debug [edit:- could this be because the baseline and service runs are calculated with a different timestep. 15 minutes is the timestep for peak management service with HVAC]. Hopefully once you have fixed the time issue you can take a look at this.

ORNLJD commented 5 years ago

@Hayden-Reeve (CC: @hlngo @emayhorn ) Thank you Hayden! I will try a clean clone version of the main library and investigate the issue with Peak Management Service.

ORNLJD commented 5 years ago

@Hayden-Reeve (CC: @hlngo @emayhorn )

Not experiencing those issues by cloning a clean copy. The reason for crashing in peak management is the test duration seems to be longer than the default length of pre-loaded weather profile. It's not crashing after fixing these two lines.

https://github.com/ORNLJD/battery_interface/blob/77c082b63869239660a3ba09dab0846db963b68d/src/fleets/HVAC_fleet/HVAC_fleet.py#L983

https://github.com/ORNLJD/battery_interface/blob/77c082b63869239660a3ba09dab0846db963b68d/src/fleets/HVAC_fleet/HVAC_fleet.py#L1046

Hayden-Reeve commented 5 years ago

@ORNLJD

That seemed to fix it. However, HVAC does not seem to be providing any P_service:

SimResults_PeakManagement_HVACFleet_20190507T0726

In addition - I have started to run refridge. I had to increase numsteps here too. When I run for reserve and peak management service there is no output . Please investigate.

SimResults_PeakManagement_RFFleet_20190507T0728

ORNLJD commented 5 years ago

@Hayden-Reeve

These most recent changes haven't been migrated to the refridge model yet. I'm working on that now. Will send the update soon.

ORNLJD commented 5 years ago

@Hayden-Reeve

Adding minor updates into the HVAC model, i.e., modify the way of calculating P_servicemax/min. Updating the commercial refrigeration model with similar modifications done for the HVAC fleets.

Regarding the conflict on src/fleet_factory.py, I have added sim_step into kwargs during the fleet instantiation.

Hayden-Reeve commented 5 years ago

@ORNLJD , let me know if and when I should retest. The main outstanding items are:

HVAC P_service is zero for service runs
Refridge not returning a Response structure

ORNLJD commented 5 years ago

@Hayden-Reeve

I have updated the code last night. Maybe you couldn't pull it due to the conflicts in the fleet_factory.py file. Could you update the HVAC and Refridge parts to the following?

elif name == 'HVAC':
    from fleets.HVAC_fleet.HVAC_fleet import HVACFleet

    # Time stamp to start the simulation
    # Please, ensure that the timestamp is the same timestamp passed at the
    # beginning of the service request
    ts = kwargs['start_time'] # Read it from kwargs dictionary
    s_step = kwargs['sim_step'] # Read it from kwargs dictionary

    hvac_fleet = HVACFleet(grid, ts, s_step)
    hvac_fleet.is_autonomous = False
    hvac_fleet.is_P_priority = True
    hvac_fleet.service_weight = kwargs['service_weight']
    if 'autonomous' in kwargs and kwargs['autonomous']:
       hvac_fleet.is_autonomous = True
    hvac_fleet.VV11_Enabled = False
    hvac_fleet.FW21_Enabled = True

    return hvac_fleet

elif name == 'Refridge':
    from fleets.Refridge_fleet.fridge_Fleet import RFFleet

    # Time stamp to start the simulation
    # Please, ensure that the timestamp is the same timestamp passed at the
    # beginning of the service request
    ts = kwargs['start_time'] # Read it from kwargs dictionary
    s_step = kwargs['sim_step'] # Read it from kwargs dictionary

    fridge_fleet = RFFleet(grid, ts, s_step)
    fridge_fleet.is_autonomous = False
    fridge_fleet.is_P_priority = True
    fridge_fleet.service_weight = kwargs['service_weight']
    if 'autonomous' in kwargs and kwargs['autonomous']:
       fridge_fleet.is_autonomous = True
    fridge_fleet.VV11_Enabled = False
    fridge_fleet.FW21_Enabled = True

    return fridge_fleet

After fixing that, could you plz try retest both the HVAC and Refridge?

Hayden-Reeve commented 5 years ago

@ORNLJD ,

I have run with the updated code but it did not address the issues (HVAC is no longer providing a response either now).

I don't have time to review incremental changes. It is not clear if you have managed to fix the issues in your version. Once you have posted results showing that you have addressed the issues mentioned above I will retest the integration.

ORNLJD commented 5 years ago

@ORNLJD ,

I have run with the updated code but it did not address the issues (HVAC is no longer providing a response either now).

I don't have time to review incremental changes. It is not clear if you have managed to fix the issues in your version. Once you have posted results showing that you have addressed the issues mentioned above I will retest the integration.

@Hayden-Reeve

OK, I will double check whether the correct version has been uploaded.

ORNLJD commented 5 years ago

@Hayden-Reeve (CC: @hlngo @emayhorn )

Finally fixed the P_response 0 issue for both HVAC and refrigeration fleets. I have got good test results for Regulation and Peak Management services.

However, I keep getting the conflicting issue for a major file inside the EV fleet. So I was unable to submit a pull request. I will send the latest code for our two fleets with test results to Hayden in a separate email. Plz restest and merge if everything looks good.

Hayden-Reeve commented 5 years ago

@ORNLJD (cc: @hlngo , @emayhorn )

Jin,

I ran both Refridge and HVAC for reserve service and the results look very good. I will test for other services later today and if I don't see any issues will try to commit the files.

Thanks for all your hard work addressing issues and providing the updated code and example results. Much appreciated.

Hayden-Reeve commented 5 years ago

@ORNLJD (@hlngo , @emayhorn @DavidWiniarski-pnnl @yliu250 @raselmahmud02 )

I have requested that the updated HVAC and Refridge codes be merged under PR #107 . Once that is done I think this request #77 can be closed. I ran a range of services and noticed some items that warrant further investigation and possibly updates. These will be best addressed in the service issue tracking and new pull requests.

Services: Reserve and Peak Management Service look to work well with HVAC and Refridge (see examples below):

20190510_all_June_events_HVACFleet

20190510_all_June_events_RFFleet

Regulation service looks good for dynamic response when shedding load. However, I would expect to see some of the fleet cycle on to increase load when required. This does not seem to happen.

20190510_August_2secnormsignals_Dynamic_HVACFleet