Mouse-Imaging-Centre / pydpiper

Python code for flexible pipeline control
Other
24 stars 10 forks source link

not enough determinism #291

Open bcdarwin opened 7 years ago

bcdarwin commented 7 years ago

Restarting the pipeline may cause a restart from the LSQ12 stages. The issue seems to be that we set a seed (in lsq12_pairwise) based on a hashable object (tuple of paths) and the hash is salted with a secret at interpreter start. One can disable this by specifying a fixed PYTHONHASHSEED, but it's a bit difficult to turn off from within a Python program (see here).

One possibility is to set a global random.seed. I didn't do this initially since this makes the lsq12 pairs chosen sensitive to the order in which they're processed, but perhaps this doesn't actually matter (if, e.g., the relevant dictionary/set/dataframe operations are deterministic). (There still seems to be some reordering of stats stages with this option if salting isn't turned off, suggesting this isn't a full fix.)

We could also create our own Random objects, but again it's a bit difficult to know what to feed them to ensure lsq12 pairs are chosen 'randomly' (but 'repeatedly') ... e.g., random.seed takes a hashable object, not an int (and only small ints hash to themselves) ...

bcdarwin commented 7 years ago

This particular case of non-reproducibility is fixed in 5a0a020b.

bcdarwin commented 7 years ago

Now affecting twolevel+MAGeT pipeline for as-yet-unclear reasons. In general this seems slightly more of an issue in version 2.x than in 1.x, perhaps due to the use of Pandas. If we can't disable all randomization somehow, perhaps we should have a feature to create/save/restore/re-run an exact pipeline graph rather than recreating it each time (this would already have performance benefits; the trick becomes telling when a pipeline is invalidated).

bcdarwin commented 7 years ago

The current issue was due to Pandas table operations being nondeterministic (maybe fixable by setting sort=True in various places?) and is fixed in develop by sorting the inputs to voxel_vote (since voting is obviously commutative/associative).