Better snakemake cluster submission support

[AroneyS]

Add rule to localrules if simple, otherwise add resources with estimated cpu/mem/time.

Not sure about all the long-reads assemblies resources.

• Add multiplier for resources with retry (if submitted job fails)
• Add proper arguments to support snakemake args
  • e.g. --profile qsub --retries 3, where profile points to ~/.config/snakemake/qsub/config.yaml which contains cluster, cluster-status, jobs, cluster-cancel and other snakemake arguments
• Add logs for each rule (stdout/stderr captured by cluster software)

[wwood]

localrules - how did I miss that before. Great.

…

On Aug 29 2023, at 12:14 pm, Rhys Newell ***@***.***> wrote: @rhysnewell approved this pull request. — Reply to this email directly, view it on GitHub (#129 (review)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAADX5A4QGOLQVYITY76KIDXXVGCHANCNFSM6AAAAAA4CEQWC4). You are receiving this because your review was requested.

More commits added, needs re-review

[rhysnewell]

Let me know when you're ready for this to be reviewed

[AroneyS]

@rhysnewell @wwood Found a bug in snakemake where it solves the dag and then exits with no error. Only happened with >1 binner with cluster-submission. After much debugging, it turns out that its because the refinery rules don't have the group: keyword. So, since cluster submission tries to combine groups, it couldn't find anything to submit.

Anyway, solution is to either add the refinery rules to group: 'binning', or remove groups entirely. I'm leaning towards the latter (or to replace them with specific grouped rules) to make the submitted jobs smaller. Was there a reason that the groups were added in the first place? I see they are split into binning/assembly/annotation/isolate...

[wwood]

Who wrote them in according to git blame? I thought they were only for cluster/cloud submission, so probably best to delete, I think, but maybe there's a reason I don't appreciate.

[AroneyS]

@rhysnewell @wwood Ran integration tests: test_short_read_recovery, test_long_read_recovery and test_short_read_recovery_queue_submission all pass.

The only issue I see now is that the refinery logs (from rosella_refinery.py) are all empty. Not sure why, since I used the same logging method as run_checkm.py, etc., and their logs are populated.

[rhysnewell]

I just had a quick look, and stdout is being written to the log file for both the run_checkm and refine_rosella rules. All logging info for rosella comes from stderr, so you'd need to pipe stderr to the log file. Stdout should generally remain empty for a rosella run. I could be incorrect though, just from a glance

[AroneyS]

The stdout and stderr should both be going to log files since stderr is redirected to STDOUT. I did that since some of the tools aren't well-behaved...

subprocess.run(
    f"checkm2 predict -i {bin_folder}/ -x {bin_ext} -o {output_folder} -t {threads} --force".split(),
    env=os.environ,
    stdout=logf,
    stderr=subprocess.STDOUT
    )

[rhysnewell]

Ah gotcha, hmm. I guess you'd have to check firstly that the log output isn't just being piped somewhere, and secondly that the refinement is even running? Like it might be getting skipped in your tests due to lack of bins or something

[AroneyS]

Hah, yeh of course. The input CheckM files are empty. I'll log that refinement is being skipped.

[AroneyS]

@rhysnewell @wwood I think its ready. Good luck reading +1191/-676 lines, at least most of them are just removing groups and adding resources/logs.

[wwood]

Hi Sam, thanks for all this work. I didn't have time to go through each and every line, but the ones I looked at were good.

Happy to see there's further testing going on too.

I've only really requested a bit more documentation, I think a short example on using the cluster profile would work well enough.

Beyond this, I think it would be worth gathering a list of advantages of Aviary over other workflows, and listing those on the README. Can you please make a start on that, maybe in another PR?

Ta

[wwood]

Unclear whether you mean the user or aviary does the creating. I think an example yml in the doco would go a long way here.

[AroneyS]

I added some stuff under Advanced Usage. Is that what you wanted?

[wwood]

Sounds good, but maybe add a subtitle rather than just advanced usage.

Also, is this still true? From that same doc. Would be worth adding a few words to say that it is only sorta true e.g. reruns in a cluster mode are requeued asking for increasingly more RAM

When performing assembly, users are required to estimate how much RAM they will need to use via -m, --max-memory, --max_memory

[AroneyS]

@wwood
The memory arg is used as a cap on job submission memory. So the job memory will increase from a set amount with each retry but stop at the cap.