search

mne-tools / mne-bids-pipeline

Automatically process entire electrophysiological datasets using MNE-Python.
https://mne.tools/mne-bids-pipeline/
BSD 3-Clause "New" or "Revised" License
140 stars 67 forks source link

BUG: Bug with caching #748

Closed larsoner closed 1 year ago

larsoner commented 1 year ago

I was just running some tests and:

  1. Ran without any derivatives (by rm -Rfing them)
  2. Changed a parameter.
  3. Ran again, and it recomputed stuff and wrote files (good).
  4. Changed the parameter back to its previous value.
  5. Ran again, and *it didn't rerun" because it said it had cached.

I think (5) is a bug -- somehow the file being there its mtime or hash or whatever didn't cause it to run again. This seems weird/bad to me. But I don't think it's too surprising since I think we only check the mtime of the input files and not the output files. Somehow we need to check to make sure that the output files haven't been overwritten (or modified) when determining if we need to rerun or not.

hoechenberger commented 1 year ago

I remember we discussed this as a known Limitation back when we implemented caching. For now, we should probably at least document this behavior

larsoner commented 1 year ago

It seems like we should be able to overcome it by:

  1. Storing and the mtime/hash of the out_files when actually running the step and returning {key: (filename, hash) ... } as the out_files dict of the step.
  2. When we do the joblib caching check, if they say the step is done/complete, we manually additionally check the hash/mtime of all the filenames and force a re-run if they've changed from what's expected.

I think it should work and not add much overhead. (Well, the minimum required amount of overhead I guess.)