GMLC-1-4-2 / battery_interface

Implemenation of Device Models and their Battery Equivalent Interface
MIT License
5 stars 0 forks source link

add distribution voltage service by NREL #43

Closed jillylikedf closed 5 years ago

hlngo commented 5 years ago

@emayhorn Can you review this PR please?

Hayden-Reeve commented 5 years ago

@jillylikedf (cc: @hlngo , @emayhorn )

Fei, I have a number of questions about running your code that would be great to discuss on the phone. Here is a summary:

  1. It is not clear which method in distribution_regulation_service should be called to run the service. Typically the main integration test.py file calls the Request_Loop method within the service, which then calls the Process_Request method (or some other method) in the device fleet (see examples below). The fleet then provides responses which are then returned back to the integration test file. I am struggling to see how that is done in your code and what is returned. (An updated test.py might help clear this up - see number 5 below).

https://github.com/GMLC-1-4-2/battery_interface/blob/eb40e28aada75ddfd04133475f3d0de4ec60e26a/src/test.py#L172-L173

https://github.com/GMLC-1-4-2/battery_interface/blob/eb40e28aada75ddfd04133475f3d0de4ec60e26a/src/services/artificial_inertia_service/artificial_inertia_service.py#L30-L46

  1. It is not clear how this service steps through time. It looks like a time-step of one hour is set (see below), however this is much longer than the 1 minute resolution of voltage parameters in the data file. I would recommend that the default timestep is suitable for the service being analyzed (1 minute or less seems appropriate). Also, typically services have used the FleetRequest function to provide time stamp and time step parameters to the fleet (as part of the fleetrequest structure - see above).

https://github.com/GMLC-1-4-2/battery_interface/blob/b2d1c5388e4ebd71f4490ed5aec2baa9368af70c/src/services/distribution_voltage_regulation/distribution_regulation_service.py#L50-L51

  1. Based on the drive cycle voltage csv file it is not clear that a non-zero P_request will be generated. The voltage range in the drive cycle is from 1.0282 to 1.0292. The upper and lower bounds of the desired voltage range are 1.04 and 0.98 so it looks like the drive cycle will not exceed these. Am I missing something?

  2. There are references to forecast methods/functions and parameters but it is not clear where they come from (note there is also a hardcoded reference to 'PV' in the code too):

https://github.com/GMLC-1-4-2/battery_interface/blob/b2d1c5388e4ebd71f4490ed5aec2baa9368af70c/src/services/distribution_voltage_regulation/distribution_regulation_service.py#L56-L62

https://github.com/GMLC-1-4-2/battery_interface/blob/b2d1c5388e4ebd71f4490ed5aec2baa9368af70c/src/services/distribution_voltage_regulation/distribution_regulation_service.py#L120-L121

  1. Finally the service test.py file seems to be for same as Peak Load Management. Can you provide the correct test.py file for this service. It may help explain many of the questions above.

https://github.com/GMLC-1-4-2/battery_interface/blob/b2d1c5388e4ebd71f4490ed5aec2baa9368af70c/src/services/distribution_voltage_regulation/test.py#L1-L22

https://github.com/GMLC-1-4-2/battery_interface/blob/eb40e28aada75ddfd04133475f3d0de4ec60e26a/src/services/peak_managment_service/test.py#L1-L22

jillylikedf commented 5 years ago

revise distribution voltage service code - updated March 25, 2019.

jillylikedf commented 5 years ago

revise distribution voltage service code - updated March 25, 2019.

Hayden-Reeve commented 5 years ago

@jillylikedf (cc: @hlngo )

Thanks for the updated version. I was able to get this to run but it needs a number of changes:

1) The initialization should not have fleet as an input (as the service factory is run before the fleet factory). Therefore:

    def __init__(self, fleet, *args, **kwargs):
        # The scope of the project is to test service with one fleet...
        self.fleet = fleet

Should be:

    def __init__(self, *args, **kwargs):
        # The scope of the project is to test service with one fleet...

2) The P and Q sensitivities are not provided by the integration test function or config file but are arguments in the input causing the code to crash. Since they are hard-coded in the service code I recommend that defaults be added. So:

    def request_loop(self, sensitivity_P, sensitivity_Q):
        # Sensitivity_P, Sensitivity_Q are values depending on feeder characteristics
        # We can use dummy value to conduct test
        sensitivity_P = 0.001
        sensitivity_Q = 0.005

becomes:

   def request_loop(self, sensitivity_P = 0.001, sensitivity_Q = 0.005):
       # Sensitivity_P, Sensitivity_Q are values depending on feeder characteristics
       # We can use dummy value to conduct test
       #sensitivity_P = 0.001
       #sensitivity_Q = 0.005

3) The fleet.process_request call needs to be modified to work correctly:

            fleet_response = self.fleet_device.process_request(fleet_request)

should be:

            fleet_response = self.fleet.process_request(fleet_request)

4) cur_time needs to be of datetime format for cur_time += delt to work correctly. Reading in the config file needs to use the parser.parse command to get the right time format. More generally the code needs to be able to run with any start time set in the main integration file. Therefore there will need to be a way for the drive cycle to run based on any start time.

5) Finally, I could not get index = list(time.values).index(cur_time) to work correctly. I thought it was because both time and cur_time were not in datetime format but this did not seem to fix it. The code works with cur_voltage set to a fixed value but this indexing will need to be fixed to run through the complete drive cycle.

Hayden-Reeve commented 5 years ago

@hlngo

Can you please reopen this PR?

Thanks Hayden

Hayden-Reeve commented 5 years ago

@raselmahmud02 ,

This PR (#43 ) has my feeback on the Voltage Regulation service.

Hayden-Reeve commented 5 years ago

81 replaces this - please close/reject

hlngo commented 5 years ago

Closed due to https://github.com/GMLC-1-4-2/battery_interface/pull/81