search

zach401 / acnportal

Research tools for the Adaptive Charging Network
BSD 3-Clause "New" or "Revised" License
75 stars 32 forks source link

battery bound #61

Closed gazzayeah closed 3 years ago

gazzayeah commented 4 years ago

Set battery charging rate between pilot signal and 0.

gazzayeah commented 4 years ago

Hi Zachary,

I apologize for my carelessness. The last line should indeed be removed and was left in the code unintentionally. I’ll make the code more concise and test it before commit.

Lots of thanks, Gary Ye

On May 1, 2020, at 15:14, Zachary Lee notifications@github.com wrote:

@zach401 commented on this pull request.

One suggestion to clean up the code. We should also write tests to make sure that the behavior you were observing is corrected by this change.

In acnportal/acnsim/models/battery.py https://github.com/zach401/acnportal/pull/61#discussion_r418759261:

     else:

charge_power = min([pilot voltage / 1000, (1 - self._soc) / (1 - self._transition_soc) self._max_power, rate_to_full]) if self._noise_level > 0:

For example we could say:

⬇️ Suggested change

  • charge_power = min(max(charge_power + np.random.normal(0, self._noise_level), 0), pilot * voltage / 1000, self._max_power, rate_to_full)
  • upper_bound = min(pilot * voltage / 1000, self._max_power, rate_to_full)
  • charge_power += np.random.normal(0, self._noise_level)
  • charge_power = np.clip(charge_power, 0, upper_bound) In acnportal/acnsim/models/battery.py https://github.com/zach401/acnportal/pull/61#discussion_r418759617:
     else:

charge_power = min([pilot voltage / 1000, (1 - self._soc) / (1 - self._transition_soc) self._max_power, rate_to_full]) if self._noise_level > 0:

  • charge_power += np.random.normal(0, self._noise_level)
  • charge_power = min(max(charge_power + np.random.normal(0, self._noise_level), 0), pilot * voltage / 1000, self._max_power, rate_to_full) We should also probably add some tests in tests/test_battery.py, to make sure this works as we expect.

In acnportal/acnsim/models/battery.py https://github.com/zach401/acnportal/pull/61#discussion_r418759887:

     else:

charge_power = min([pilot voltage / 1000, (1 - self._soc) / (1 - self._transition_soc) self._max_power, rate_to_full]) if self._noise_level > 0:

  • charge_power += np.random.normal(0, self._noise_level)
  • charge_power = min(max(charge_power + np.random.normal(0, self._noise_level), 0), pilot * voltage / 1000, self._max_power, rate_to_full)

    ensure that noise does not cause the battery to violate any hard limits.

    charge_power = min([charge_power, pilot * voltage / 1000, self._max_power, rate_to_full]) With the new addition, this line is redundant.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zach401/acnportal/pull/61#pullrequestreview-404425819, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJWKZ6FK3UDUPXFIHUH4KMLRPNCTPANCNFSM4MXJOSKA.

zach401 commented 4 years ago

Hi Gary,

Don't worry about it! Let me know if you need any help with writing tests. Thanks again for finding this bug and fixing it. Glad to have you contributing to the project.