pepkit / looper

A job submitter for Portable Encapsulated Projects
http://looper.databio.org
BSD 2-Clause "Simplified" License
20 stars 7 forks source link

completed flag check may erroneously stop on regex matches #470

Closed nsheff closed 3 weeks ago

nsheff commented 4 months ago

I'm trying to submit 6 jobs with looper. I've never submitted any before, it's a brand new project. I noticed one of them says:

Found existing status: completed. Skipping sample.

This is bizarre because it's a brand new project! It has never been submitted before.

I realize this sample shares a prefix with another sample: one is named pairs_swap_maintain_coords, which the pipeline runs, and then the next sample is named pair_swap -- which the pipeline incorrectly says is already completed.

I'm guessing there's a regex that's looking for {sample_name}*_completed.flag -- if that's the case, it would actually register the first one as completed for the second one, and then never submit that job.

nsheff commented 4 months ago

This may actually be a bug with pipestat, rather than looper. I'm guessing this coincides with a switch to using pipestat for status checks.

donaldcampbelljr commented 4 months ago

I can't reproduce this using a modified hello_looper example for both the basic and the pipestat approaches (which look for their flags in slightly different ways).

donaldcampbelljr commented 4 months ago

However, looking at the basic, non-pipestat example, I can see where the function fetch_sample_flags might have issues if you had a flag from a different sample in the results folder, because of this logic: https://github.com/pepkit/looper/blob/1468956dde66abf5b853c80eaeaee2d411bfad64/looper/utils.py#L93-L98

Appears it is only concerned with .flag and the pipeline_name. The sample name doesn't matter.

nsheff commented 3 months ago

was this fixed by the pipestat update referenced above?

donaldcampbelljr commented 3 months ago

I don't believe so. The pipestat code above was broken for filebackend and is not used for getting sample statuses.

donaldcampbelljr commented 3 months ago

Should be solved with the above commit.