popsim-consortium / analysis2

Analysis for the second consortium paper.
8 stars 14 forks source link

Vaquita genetic map snag #111

Closed silastittes closed 4 months ago

silastittes commented 4 months ago

The pipeline as written seems to require a genetic map at various places. I'm getting the following error here :

ERROR:snakemake.logging:RuleException:
ValueError in file analysis2/workflows/n_t.snake, line 488:
GeneticMap 'PhoSin/None' not in catalog ()

I think the issue is most salient in this function, where various aspects of the code depend on GeneticMap methods.

I'm wondering if folks more familiar with stdpopsim can help me navigate this. Maybe a silly solution, but maybe an update to the catalog code that makes a constant genetic map file that can be downloaded so that the GeneticMap methods are still available? Maybe this is already the case and I just need to adjust how to initial the pipeline? Thanks for thoughts!

andrewkern commented 4 months ago

tagging @petrelharp here -- is there no PhoSin map? maybe there is a map that needs to be uploaded to AWS?

petrelharp commented 4 months ago

No, there is no genetic map (perhaps unsurprisingly?) Screenshot from 2024-05-09 17-14-48

Sounds like the pipeline needs to be modified - there is no reason we need to use a map?

silastittes commented 4 months ago

I don’t think we need one for the aims of the manuscript. Could we make a “fake” constant one for Vaquita for this project ? My initial impression is it will take a lot of refactoring to make the pipeline work without one, but that may reflect my lack of familiarity with the code bases.

petrelharp commented 4 months ago

Well, probably? That sounds like the long way round but then I'm not familiar with the pipeline. The error you got could be fixed relatively easily with some special-case logic, but I haven't seen how often this comes up?

silastittes commented 4 months ago

Yeah, it may be a smaller issue than I initially thought. I’ll try making some minor edits and see if I can get it working without a genetic map.

silastittes commented 4 months ago

Ok, I think I got this sorted out -- PR forthcoming. MSMC is failing to converge, but maybe that won't be an issue for the production pipeline?

andrewkern commented 4 months ago

guessing MSMC isn't converging in the tiny config cause of too few sites?

silastittes commented 4 months ago

I think so too. Giving both species on more clean run with tiny config to make sure everything is square then I’ll PR the update.

silastittes commented 4 months ago

fixed with #112