search

microsoft / jericho

A learning environment for man-made Interactive Fiction games.
GNU General Public License v2.0
253 stars 42 forks source link

Valid actions ctypes #50

Closed mhauskn closed 2 years ago

mhauskn commented 2 years ago

Creates a ctypes alternative for the valid action search.

mhauskn commented 2 years ago

Thank you Marc for the corrections - I've implemented both in the latest push! The speedups from your parallelization look quite nice - can you make a PR to include them?

A benchmark which would be valuable with the parallelization is the multi-environment use case. E.g. using a vectorized environment setup like DRRN as well as Redis DB-world-state-hashing. In this case, I suspect the speedups from parallelization will be less than the single-environment case, but hopefully better than no parallelization. Let me know what you think.

MarcCote commented 2 years ago

Done. See my PR to your branch.

Regarding "vectorized environment setup like DRRN", that's totally an issue since VecEnv uses daemon processes and those processes can't have children, i.e. can't use mp.Pool to compute the valid actions.

I found a workaround using concurrent.futures.ProcessPoolExecutor but that was way slower (probably some issue with the Python GIL or something). :(

MarcCote commented 2 years ago

Did some additional benchmarking. Using concurrent.futures.ProcessPoolExecutor for the outer parallelization loop (i.e., VecEnv) but using the multiprocessing.Pool for Jericho works great.

MarcCote commented 2 years ago

I've updated PR #51.

mhauskn commented 2 years ago

This PR looks ready to go from my end. One question is whether we would want to enabled the ctypes + parallelization of valid actions by default? I suspect that it should help in most cases - what do you think @MarcCote?

MarcCote commented 2 years ago

I had a similar thought. I would say yes, make the parallel (which uses ctypes) version the default. If we see that it breaks at some point, we can revise this decision.