Why is calling `init_args_and_threads` multiple times from the same process not recommended?
ankeshanand opened this issue · 6 comments
Hi,
I need to initialize the coinrun library multiple times in the file (separately for train / test), and I was able to do so without any visible issues. I did notice though that it's mentioned in the function doc of init_args_and_threads to not do this.
Lines 95 to 97 in 601de52
Is there a reason behind this? (Trying to understand why that is the recommendation to avoid any unknown issues)
Well it's not threadsafe for one, so if you called in different threads at the same time, I'm not really sure what would happen.
Should you call it multiple times in a single thread? Well if you call it with the same global args each time, maybe nothing bad would happen, especially if you didn't create the environment until after you called it multiple times.
So you said you inited it multiple times for train/test, which means that, if both are in the same process, it's likely either your test environment is the same as your train environment or vice versa since it is using global state (for instance USE_HIGH_DIF). If that's not an issue for you, carry on. If it is an issue, I'd recommend running two different processes. You can even isolate just the environment in a subprocess, like how SubprocVecEnv works, though I don't think we have anything that does that currently.
Thanks, I was re-initing Coinrun when switching b/w train and test to have the right global state, but you're right thread safety might be a problem. I will try isolating them within separate sub-processes, thanks for the tip!
I wrote a quick wrapper to wrap a coinrun env inside a subprocess (pretty similar to SubprocVecEnv). https://gist.github.com/ankeshanand/24d6f45d17cc44a9e0c9d71a636d537e
Nice! Is there a reason it doesn't just use SubprocVecEnv?
This is pretty similar to SubprocVecEnv albeit some minor differences.
From what I got, SubprocVecEnv expects a list of env_fns and converts them into a VecEnv. In this case, we already have a VecEnv (and thus don't need the unsqueeze when doing step) that we want to wrap in a subprocess.
Oh right, unfortunate. There's some functionality in the multiprocessing module to do some of this already, but it seems to not handle properties well at the very least.