search

elsasserlab / minute

MINUTE-ChIP data analysis workflow
https://minute.readthedocs.io
MIT License
2 stars 0 forks source link

Remove dynamic rule naming #163

Closed cnluzon closed 1 year ago

cnluzon commented 1 year ago

As mentioned in #159 this is making the workflow crash in recent versions of snakemake. I have not been able to find out the reason behind this, on the test data it only crashes with the rule that is renamed to compute_scaling_factors_group_group2 even though it is successfully renamed on the job table that is printed at the beginning:

Job stats:                                                                                                                                                                                                                                                    
job                                     count    min threads    max threads                                                    
------------------------------------  -------  -------------  -------------                                                    
compute_scaling_factors_group_group1        1              1              1                                                                                                                                                                                   
compute_scaling_factors_group_group2        1              1              1

I still get:

Building DAG of jobs...
UnknownRuleException:
There is no rule named compute_scaling_factors_group_group2.

This happens only in more recent versions of snakemake, with snakemake-minimal 7.16.0 it works (current version being 7.22.0 and still crashing). The reason could be some changes in how snakemake is handling the workflow DAG that I cannot easily predict, however I do not think it is a snakemake bug.

Maybe accessing and changing the workflow object is not the best approach to achieve this, my suggestion for the sake of stability is removing the code that renames the rules and let the workflow handle the rules as anonymous, since the impact of this is mostly cosmetic and not so relevant for the general use of the pipeline.

I wonder if there is a better solution to come to avoid creating rules inside a for loop, but I since input/outputs are depending on scaling groups, that might just not be the case.

Fixes #159

marcelm commented 1 year ago

I agree this was fine as long as it worked, but if there are crashes, it’s clear it should go.