Electrolyzer and FuelCell are ready for integration test

[hlngo]

@Hayden-Reeve @emayhorn I have merged the 2 fleets in. My initial test with both was half way through but didn't complete. Can either of you run&review the result? If there is anything, please create an issue for it. Thanks.

[Hayden-Reeve]

@emayhorn , @hlngo , I will be travelling back from Richland today but can look at it tomorrow.

Was there a particular service you were running? If not I will work Electrolyzers with AI.

[hlngo]

@Hayden-Reeve Yes, please start with AI service. If you have time, it's great if you can test it with other services as well.

[Hayden-Reeve]

@hlngo , @emayhorn

See my comments on #63 - the electrolyzer seems to be missing the frequency_watt method and does not call 'get_frequency'.

I also ran electrolyzer for regulation. That worked and the results looked reasonable.

[Hayden-Reeve]

@hlngo , @rkadavil ,

Is there a way to tell what services Electrolyzer and Fuelcell are ready for? Just want to make sure I don't get out in front of the development by testing on services before they are ready.

Regards Hayden

[hlngo]

@Hayden-Reeve No unless @rkadavil did some integration test himself.

[rkadavil]

@Hayden-Reeve No unless @rkadavil did some integration test himself.

Based on our weekly discussions, we had agreed that only Electrolyzer model will be coded for services. At the moment, we are still working to integrate the Electrolyzer model with the get_frequency method.

[Hayden-Reeve]

@rkadavil ,

When testing Electrolyzers with the unit test (PR #73 ) it appears that electrolyzers will crash with P_request = 0 in the lines below.

https://github.com/GMLC-1-4-2/battery_interface/blob/d0dd3ec218910defc31e4551e096d560866aa670/src/fleets/electrolyzer_fleet/ey_fleet.py#L157-L160

[rkadavil]

@rkadavil ,

When testing Electrolyzers with the unit test (PR #73 ) it appears that electrolyzers will crash with P_request = 0 in the lines below.

battery_interface/src/fleets/electrolyzer_fleet/ey_fleet.py

Lines 157 to 160 in d0dd3ec

if self.inc == 0: self.ey_Ne = Preq/self.ey_E_size cprint("No. of Electrolyzers used is:"+"\t"5+"%d" % self.ey_Ne, 'green') Pr = 1e3Preq/self.ey_Ne

Hi @Hayden-Reeve These lines were already removed in the updated pull request PR#63 for Electrolyzer and FuelCell (Commit# 9b3e6dc). I tested Electrolyzer to run only with P_request = 0 and it didn't crash. It however failed when I ran with P_request = None and this has been fixed in the latest commit for both the modes which is listed here: 3343b38

[rkadavil]

@Hayden-Reeve I ran the unit_test.py code for Ey and FC (Commit #3343b38). It failed at assertIsNotNone for 11 variables: C, Eff_discharge (for Electrolyzer), Eff_charge (for FuelCell), Q_dot_up, Q_service, Q_service_max, Q_dot_down, Q_togrid_max, Q_togrid_min, dT_hold_limit, SOC_cost, Strike_price as they have been set to None in the models. Some of these variables are not applicable to the models. For e.g. FuelCell can only discharge so Eff_charge doesn't apply and similarly, Electrolyzer can only charge. There is no Q being provided by Fuel cell. So, should we set these responses to 0 or None in the device models?

[Hayden-Reeve]

@rkadavil

We will discuss with the service owners tomorrow to get their input on what level of definition is needed. For now I was checking that the API variables exist (so even getting the assert fail warning is a good sign). No action needed on test output at this stage.

Sorry for missing PR #63 . Good to know you can handle 'none' as well. It would be good to create formal tests for these too.

[rkadavil]

@Hayden-Reeve Could you kindly modify line 64 in fleet_factory.py to remove True from ElectrolyzerFleet class arguments? The True positional argument tries to fit the entire P_request curve. It should be: fleet = ElectrolyzerFleet("", "config.ini", "Electrolyzer")

[Hayden-Reeve]

@hlngo ,

Can you address this request? I think it would be faster for you to do it and I think you are more familiar with that integration.

[hlngo]

@Hayden-Reeve Sure

[Hayden-Reeve]

@rkadavil (cc: @hlngo , @emayhorn )

I have tested the electrolyzer fleet against three services (artificial inertia, reserve, and regulation). In short it works for those services. Below are some recommendations and suggestions:

1. The pull request (#63 ) includes files from other fleets (60 files in total) making it hard to pull this request locally and not have merge errors and alter other files. In some cases I updated files manually to get around issues - so I hope I am testing with the latest. Recommend you update (or resubmit) your pull request to include only the files you want changed.

2. It is important in the fleet factory that the autonomous mode flag defaulted to 'false' and is set when in 'Artificial Inertia' mode. I had some issues running the various services before this was resolved:

    elif name == 'Electrolyzer':
        from fleets.electrolyzer_fleet.ey_fleet import ElectrolyzerFleet
        fleet = ElectrolyzerFleet("", "config.ini", "Electrolyzer")

        fleet.is_autonomous = False
        if 'autonomous' in kwargs and kwargs['autonomous']:
           fleet.is_autonomous = True
        fleet.VV11_Enabled = False
        fleet.FW21_Enabled = True
        return fleet

3. Artificial Inertia service ran but did not generate any response variation with frequency (as would be expected). Below is my result (running with the latest service code from #75 . Is this the response you expect? This may be an issue from item 1 above.

4. The device impact metric spreadsheet looks good. I would recommend a row at the top that provides the summation/average for the entire time period. It was also not clear how to get this file to write out. It did not automatically write for the runs I executed. Again, this may be tied to issue item 1 above.

[rkadavil]

@Hayden-Reeve I've highlighted my responses in bold.

@rkadavil (cc: @hlngo , @emayhorn )

I have tested the electrolyzer fleet against three services (artificial inertia, reserve, and regulation). In short it works for those services. Below are some recommendations and suggestions:

1. The pull request (#63 ) includes files from other fleets (60 files in total) making it hard to pull this request locally and not have merge errors and alter other files. In some cases I updated files manually to get around issues - so I hope I am testing with the latest. Recommend you update (or resubmit) your pull request to include only the files you want changed.

I'm sorry about that. I had requested @hlngo to either reject PR#63 or only approve the files from our Electrolyzer and FuelCell models. Would it be okay if you or @hlngo can close or reject PR#63 and then I can create a new PR that only includes the changes for our device models?

2. It is important in the fleet factory that the autonomous mode flag defaulted to 'false' and is set when in 'Artificial Inertia' mode. I had some issues running the various services before this was resolved:

    elif name == 'Electrolyzer':
        from fleets.electrolyzer_fleet.ey_fleet import ElectrolyzerFleet
        fleet = ElectrolyzerFleet("", "config.ini", "Electrolyzer")

        fleet.is_autonomous = False
        if 'autonomous' in kwargs and kwargs['autonomous']:
           fleet.is_autonomous = True
        fleet.VV11_Enabled = False
        fleet.FW21_Enabled = True
        return fleet

I'm not sure if I followed you correctly on this. Do you mean to say that the is_autonomous flag should be set to False by default? if that's the case, then config.ini file already has that variable set to False:

# Fleet configuration
is_P_priority = False
FW21_Enabled = True
is_autonomous = False

Also, the first positional argument in ElectrolyzerFleet class expects the Grid_Info_data_artificial_inertia.csv to be provided. With empty quotes, the device model runs with no services enabled. So, the fleet factory code for Electrolyzer needs to be modified as shown.

# Use instantaneous power request + frequency regulation
# Autonomous frequency request can also be changed from config.ini
grid_dat = GridInfo('Grid_Info_data_artificial_inertia.csv')
fleet = ElectrolyzerFleet(grid_dat, "config.ini", "Electrolyzer")

This is shown as SCENARIO 3 on lines 211-216 in the test.py file. Commit#3343b38b9 in PR#63

1. Artificial Inertia service ran but did not generate any response variation with frequency (as would be expected). Below is my result (running with the latest service code from #75 . Is this the response you expect? This may be an issue from item 1 above.

Your result looks different than what we have seen and might be due to the earlier points that I've highlighted. I've attached the frequency response when we run AI on Electrolyzer Please note, in our case, we can only adjust our power consumption since the Electrolyer is a load. The equations for freq. regulation as defined in frequency_droop.py, we assume, is for sources that dictates frequency and can inject power into the grid and so will not work satisfactorily for Electrolyzer.

1. The device impact metric spreadsheet looks good. I would recommend a row at the top that provides the summation/average for the entire time period. It was also not clear how to get this file to write out. It did not automatically write for the runs I executed. Again, this may be tied to issue item 1 above.

That's odd. The impact metrics file is generated with current datetime stamp appended to the file name by the output_metrics method and this is called after the requests has been processed by the process_request method. The output_metrics method is highlighted on lines 346-342 in ey_fleet.py in the latest commit Commit#3343b38b9 in PR#63 The implementation is shown in test.py on line 183 in the latest commit Commit#3343b38b9 in PR#63

[Hayden-Reeve]

@hlngo , can you please either reject PR #63 or accept only the relevant parts (I don't think I have permission to do this).

@rkadavil , let's step through the issues above once we have a cleaner set of code. Some of these issues may be due to mismatches between integration code and the electrolyzer fleet code.

[Hayden-Reeve]

@rkadavil , @hlngo (cc @emayhorn )

I ran #78 (with #73 , #75 and the fixes identified in #79 ) and a cleaned cloned copy of the battery_interface library. I still get many of the issues above when I run electrolyzer with reg_service, reserve, and artificialinertia. Are you testing your devices with the main integration code (test.py) and the latest clean clone of the library? If not I would recommend that. Here is a summary of the issues I am seeing:

1. ey_fleet.py does not successfully set the config.ini settings. I have no idea why this is - I am using version 3.7.4 of config parser. For example:

https://github.com/GMLC-1-4-2/battery_interface/blob/28f039ca508149789646f244799a17c2229035b6/src/fleets/electrolyzer_fleet/ey_fleet.py#L112

sets is_autonomous to True despite the default being False and config being false. Regardless, there needs to be a way to set this to true for the autonomous mode so I have made these changes in fleet factory. This allows the other cases to run and ensures the correct inputs to the ElectrolyzerFleet.

I noticed in your test file that grid is only passed in for autonomous mode but not for other modes. This is counter to how other fleets do it - as other services need grid. Maybe fleetfactory needs to be updated with if statements to make the right calls. Alternatively and preferred would be for the ElectrolyzerFleet method to have constant inputs and the inputs change based on need. I have been testing with this structure in fleetfactory:

    elif name == 'Electrolyzer':
        from fleets.electrolyzer_fleet.ey_fleet import ElectrolyzerFleet
        fleet = ElectrolyzerFleet(grid, "config.ini", "Electrolyzer")
        fleet.is_autonomous = False
        if 'autonomous' in kwargs and kwargs['autonomous']:
           fleet.is_autonomous = True
        fleet.VV11_Enabled = False
        fleet.FW21_Enabled = True
        return fleet

2. With the changes above (and some bug fixes in the service) the code runs for the three services but only seems to provide an interesting result for reg_service. The others have constant results. I have not had time to dig into why this is happening:

2. Services are not calling the output_metrics method. I recommend this is discussed at the next meeting so it is clear who has responsibility to call this function (devices, services, or the integration test)?

3. Add P_base to the outputs in the next update as well.

I am traveling most of next week and won't have much more time to look at this. I recommend you try running with the main integration file (src\test.py) and let us know what inputs should be going into ElectrolyzerFleet in fleetfactory based on the service type. I suspect this is were the issue is.

[rkadavil]

Hi @Hayden-Reeve, please see my responses in bold:

@rkadavil , @hlngo (cc @emayhorn )

I ran #78 (with #73 , #75 and the fixes identified in #79 ) and a cleaned cloned copy of the battery_interface library. I still get many of the issues above when I run electrolyzer with reg_service, reserve, and artificialinertia. Are you testing your devices with the main integration code (test.py) and the latest clean clone of the library? If not I would recommend that. Here is a summary of the issues I am seeing:

This has been addressed in latest PR #91

1. ey_fleet.py does not successfully set the config.ini settings. I have no idea why this is - I am using version 3.7.4 of config parser. For example:

battery_interface/src/fleets/electrolyzer_fleet/ey_fleet.py

Line 112 in 28f039c

self.is_autonomous = bool(self.config.get(mdl_type, "is_autonomous", fallback=False)) sets is_autonomous to True despite the default being False and config being false. Regardless, there needs to be a way to set this to true for the autonomous mode so I have made these changes in fleet factory. This allows the other cases to run and ensures the correct inputs to the ElectrolyzerFleet.

I noticed in your test file that grid is only passed in for autonomous mode but not for other modes. This is counter to how other fleets do it - as other services need grid. Maybe fleetfactory needs to be updated with if statements to make the right calls. Alternatively and preferred would be for the ElectrolyzerFleet method to have constant inputs and the inputs change based on need. I have been testing with this structure in fleetfactory:

    elif name == 'Electrolyzer':
        from fleets.electrolyzer_fleet.ey_fleet import ElectrolyzerFleet
        fleet = ElectrolyzerFleet(grid, "config.ini", "Electrolyzer")
        fleet.is_autonomous = False
        if 'autonomous' in kwargs and kwargs['autonomous']:
           fleet.is_autonomous = True
        fleet.VV11_Enabled = False
        fleet.FW21_Enabled = True
        return fleet

I will check the models against test.py code and debug. I've also attached the result for Electrolyzer running frequency regulation. Since Electolyzer is a load, it reduces its P_response as frequency goes down and increases P_response as frequency goes high. Please check PR #91

1. With the changes above (and some bug fixes in the service) the code runs for the three services but only seems to provide an interesting result for reg_service. The others have constant results. I have not had time to dig into why this is happening:

I do not follow this line: Services are not calling output_metrics method. The output_metrics method can be called for Ey and FC once the process_request is done processing the requests. This can be called using the fleet.output_metrics(string filename) after the fleet is initialized.

1. Services are not calling the output_metrics method. I recommend this is discussed at the next meeting so it is clear who has responsibility to call this function (devices, services, or the integration test)? This has been addressed in latest PR #91
2. Add P_base to the outputs in the next update as well.

I am traveling most of next week and won't have much more time to look at this. I recommend you try running with the main integration file (src\test.py) and let us know what inputs should be going into ElectrolyzerFleet in fleetfactory based on the service type. I suspect this is were the issue is.