NREL / hive

HIVE™ is a mobility services research platform
https://nrelhive.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
22 stars 9 forks source link

Rjf/determinism #203

Closed robfitzgerald closed 1 year ago

robfitzgerald commented 1 year ago

some efforts here to address non-determinism in hive. still seeing it after these changes. for now,

any thoughts on what else i could do here?

edit: i just went through and added a ton more to this. details:

i was able to observe 50 denver_demo runs in a row that had the same result. i'm also seeing those runs take 15 seconds which i believe is a bit longer than before. we may see a bigger performance hit with larger runs.

note: when i run hive at the command line twice in a row, i'm still seeing different results still. maybe we're not controlling for randomness as well when calling it there for some reason. to provide an example, i've added examples/test_for_determinism.py which runs a quick set of 5 iterations of denver and confirms whether all results match or not.

robfitzgerald commented 1 year ago

@nreinicke i think i've got it! can you run python examples/test_for_determinism.py in your hive conda env and confirm that everything comes out "good" after 5 iterations of denver demo? just checking that high-level stats like soc and vkt are exactly the same, but i figure that should be good enough to confirm we're getting deterministic runs.

nreinicke commented 1 year ago

Just pulled and ran and it looks like it's checking out!

finished iteration 4
mean_final_soc is good, all values match
requests_served_percent is good, all values match
total_vkt is good, all values match
total_kwh_expended is good, all values match
total_gge_expended is good, all values match
total_kwh_dispensed is good, all values match
total_gge_dispensed is good, all values match

What was the main cause?

robfitzgerald commented 1 year ago

maybe a terrible idea, but, i set up the github actions to call the determinism test at PR. it will run denver demo twice and confirm high-level metrics match. that'll slow us down a little but maybe worth it to make sure we don't backslide on determinism.

robfitzgerald commented 1 year ago

What was the main cause?

i went beyond SimulationState and did a search for .items(), .values(), .keys() and sorted(... across the codebase. there were a number of places where we were sorting by something without a fallback. i think a big one was sorting ChargeQueueing agents by queue time. copying the example i added here:

vs: List[Vehicle] = ... #
sorted(vs, key=lambda v: v.distance_traveled_km)          # bad
sorted(vs, key=lambda v: (v.distance_traveled_km, v.id))  # good

that's a problem if two vehicles had traveled the same distance, there's nothing to determine what order those two will fall into. gets worse when the sort is by "queue time", those are going to be integers, and so anytime two agents have the same queue time, there's no tiebreaker to use as a second-tier sort. i went through and added id sorts in those cases.

a similar problem would happen with calls to get_{vehicles|stations|bases|requests} methods that supplied sort_keys. and then there were just a few other iterations that didn't sort by id in absence of any requested sort values.

tl;dr: sortsortsortsortsortsortsortsortsortsortsortsortsortsortsort

nreinicke commented 1 year ago

Aha that makes a lot of sense, nice work getting deep into this one to figure it out!!

With respect to the determinism test in the action I think that's a good idea.

robfitzgerald commented 1 year ago

@nreinicke the determinism check passed, yay! it replaces the 'hive denver_demo.yaml' action. ready for your review.