search

GMLC-1-4-2 / battery_interface

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

Distributed Voltage Regulation #81

Closed raselmahmud02 closed 5 years ago

raselmahmud02 commented 5 years ago

This update includes the following:

  1. Battery equivalent API for distributed voltage regulation
  2. Bugs fixed to make the code operational
  3. Start_time can be passed as argument
  4. Reports service impact metrics in a csv file
  5. Added test.py file for this service
  6. Tested with PV
Hayden-Reeve commented 5 years ago

@raselmahmud02 (cc: @hlngo , @emayhorn )

Thanks for the new pull request for Voltage Regulation. It looks much better and appears to be almost done. Below are a list of recommendations (some in scope and some general comments for the overall library and likely out of scope). I also have made changes to src\test.py and service_factory.py to integrate this service and have committed those in #84 .

  1. Your pull request has this contribution in the folder: "battery_interface\src\services\artificial_inertia_service\distribution_voltage_regulation" Please contribute it in: "battery_interface\src\services\distribution_voltage_regulation" So it is a standalone service and not part of AI.

  2. The impact_metrics.csv file is saved in the src\ folder. Based on the amount of input we will generate (see below) we will want to save it in a separate folder. I am not sure if we have a standard approach to this. The best place is probably: battery_interface\src\integration_test\distribution_voltage_regulation

  3. Please also save the plots to this area. This will be very useful when we are doing multi-runs and want to save and then review a large number of results (by device or time).

  4. File Names: We are recommending that each service save results with a file name that captures the date they were run, the fleet name, and the service/metric/plot name. (we are assuming the start-time etc will be included in the plot or csv data). This will ensure that we don't over-write files when we do device multi-runs. Here is an example: https://github.com/GMLC-1-4-2/battery_interface/blob/d4fc77448f0b3ee2c62b7f999e0a281548fb4bd5/src/test.py#L139-L146

  5. The impact metrics has a spelling mistake (metris versus metrics)

  6. I am not sure if the results are actually sensible. We will have to review these with Ebony and device owners to review the results and understand if and how to apply fleet scaling and if the metrics csv format is correct.

  7. Finally (and out of scope) I noticed that you use maya for time parsing etc. Others use dateutil. In the far future we may want to consider consolidating the packages used across the library. But lets not worry about that now.

raselmahmud02 commented 5 years ago

Following updates were made in this pull request:

  1. distribution_voltage_regulation moved to 'battery_interface\src\services\distribution_voltage_regulation'
  2. location to save the *.csv file is set to 'battery_interface\src\integration_test\distribution_voltage_regulation' 3.location to save the plot files is set to 'battery_interface\src\integration_test\distribution_voltage_regulation'
  3. file name is updated with recommended format
  4. spelling correction