Closed robfitzgerald closed 1 year ago
clintonsteiner
commented
1 year ago Hey @robfitzgerald would you mind helping me write a test to illustrate this behavior? Then I think we could write something to solve this
Wondering if we could skip the iteration while the delta it creates is below a certain percentage, then if its due to grid throttle, when the grid is unthrottled it would resume charging? Or in the case of the battery being full, it would just continue to skip
robfitzgerald
commented
1 year ago Wondering if we could skip the iteration while the delta it creates is below a certain percentage
i'm not sure if a skip makes sense here, because then at the i+1th iteration, it would just "try again" with nothing else changed. i like the idea of a practical threshold of some very low increment, but, as stated above, we also can capture this effect by comparing the mechatronics.fuel_source_soc(charging_vehicle) to the mechatronics.fuel_source_soc(updated_veh). if it's equal, we break.
as for reproducing it, the problem here is a DC fast charger, BEV mechatronics, default powercurve, with a 100% battery charge target. that should produce this result:
from nrel.hive.resources.mock_lobster import *
from nrel.hive.model.vehicle.mechatronics.powercurve.powercurve_ops import time_to_full
bev = mock_bev(battery_capacity_kwh=50)
vehicle = mock_vehicle(soc=0.99) # ...almost full
charger = mock_dcfc_charger()
# should throw stack overflow:
time_to_full(vehicle, bev, charger, target_soc=1.0, sim_timestep_duration_seconds=60)edit: yuppers:
>>> time_to_full(vehicle, bev, charger, target_soc=1.0, sim_timestep_duration_seconds=60)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/powercurve/powercurve_ops.py", line 35, in time_to_full
time_charged = _fill(vehicle)
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/powercurve/powercurve_ops.py", line 33, in _fill
return _fill(updated_veh, updated_time_charged_acc)
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/powercurve/powercurve_ops.py", line 33, in _fill
return _fill(updated_veh, updated_time_charged_acc)
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/powercurve/powercurve_ops.py", line 33, in _fill
return _fill(updated_veh, updated_time_charged_acc)
[Previous line repeated 989 more times]
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/powercurve/powercurve_ops.py", line 29, in _fill
updated_veh, time_delta = mechatronics.add_energy(
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/mechatronics/bev.py", line 235, in add_energy
updated_vehicle = vehicle.modify_energy(
File "/Users/rfitzger/dev/nrel/hive/hive/nrel/hive/model/vehicle/vehicle.py", line 154, in modify_energy
return replace(self, energy=energy)
File "/Users/rfitzger/anaconda3/envs/hive/lib/python3.9/dataclasses.py", line 1284, in replace
return obj.__class__(**changes)
File "<string>", line 3, in __init__
RecursionError: maximum recursion depth exceeded while calling a Python objectThanks for the response, in my head I would assume we would need to continue evaluating if its charging or not indefinitely to get around the grid throttle scenario.
How would you feel about breaking if we are within some percent of full and the delta change is low enough?
robfitzgerald
commented
1 year ago in my head I would assume we would need to continue evaluating if its charging or not indefinitely to get around the grid throttle scenario
that's an interesting point. at first my answer was going to be "this function is used in estimating the expected duration of a charging session during charging session dispatch where we have more relaxed correctness requirements". but, you're right, assuming the grid was throttled 100% for 10 minutes, we want the logic to be correct for both dispatch and the underlying simulation physics. thanks for the suggestion.
do you think we have gotten around the stack overflow issue? we could replace the iteration from recursion to using a fold (ft.reduce) to protect against very long grid blackouts.
sorry for the delay, i was in a meeting 🕴️
robfitzgerald
commented
1 year ago as for what to use for the threshold; that may require some empirical playing-around-with the arguments to Mechatronics.add_energy (or, a PhD in Python floating point number behaviors).
ideally we also add this to the config, perhaps in the DispatcherConfig class, with your sensible default value. though i can imagine we may need to add similar logic to the vehicle_state_ops.charge method. @nreinicke should this go in Sim instead for that reason?
clintonsteiner
commented
1 year ago I don't have a ton of experience with ft.reduce but I think switching out of the recursion model for this is going to be required or else we will keep hitting this recursion depth limit
nreinicke
commented
1 year ago should this go in Sim instead for that reason?
yeah I think that makes sense.
The grid throttle in an interesting edge case I hadn't considered. This is tricky since that would assume we have perfect knowledge of when the grid might throttle but this is really meant to be an estimate since we probably won't have perfect knowledge of the grid.
robfitzgerald
commented
1 year ago switching out of the recursion model for this is going to be required or else we will keep hitting this recursion depth limit
yea. and actually, to make this a fold/reduce, we would need to make an overly-complex iterator that encapsulates the stopping condition. i think it's just not pythonic to do this without side effects, such as a while loop, which is a bummer, but stack overflows are an even bigger bummer, so maybe it should be a while loop.
clintonsteiner
commented
1 year ago Opened a pull request here: https://github.com/NREL/hive/pull/173 Seems like it is working ok and iterates a couple hundred times before giving up.
The grid throttle scenario will certainly affect this logic though, might want to put that in assumptions Happy to do it if you point me in the right direction
robfitzgerald
commented
1 year ago added the comment on your PR that we know the maximum time to test, it's always (sim.end_time - sim.sim_time) / sim.timestep_duration_seconds. that could be piped in from the call site of this function, which at least prevents an infinite loop (since end_time is finite in HIVE)
nreinicke
commented
1 year ago Closed by #173
problem
powercurve_ops.time_to_fill estimates the time to charge a vehicle by micro-simulating the charge session until the target SoC is reached. but it is possible for the charger/mechatronics combination to prevent reaching this target SoC value, and because the function does not check for incremental improvement, it leads to an infinite loop which terminates with a stack overflow from reaching the max function recursion depth.
where this happens can be illustrated with this example. the vehicle is a leaf_50. the agent is headed home and finds a DCFC charger. the charging session begins with the vehicle at 44.08 kWh. the human_driver_off_shift_charge_target (by default) is 100%. the charging session is extremely slow since the powercurve tapers off near the 100% mark:
at around 4 hours, we stop seeing any effect from charging and this leads to the stack overflow (see log section, below). i'm guessing this has somewhat to do with floating point arithmetic precision and the charge tapering table.
fix
two ideas.
first, this function could check if the previous kWh == the next kWh and if so, cancel. this is probably ok for a function that has the purpose of estimation, but i could see where this approach would fail more generally, such as when we implement grid throttling. in the case the grid momentarily ceases charge sessions, this would be mis-interpreted as "finishing a session" by this function.
second, we could modify the powercurve to have a lower limit to output values. if you look at the chart above, it gets really close to zero, or maybe even reaches zero. that's a bit nonsensical for our purposes, even if it is correctly modeling the physics. if we enforced a lookup to always produce a value that is at least 1% for instance, then we shouldn't run into this problem. this may be a better solution.
log
i stubbed the recursive function to show it at work and include the error message displayed.