tskit-dev / pyslim

Tools for dealing with tree sequences coming to and from SLiM.
MIT License
27 stars 23 forks source link

Issues with recapitate tutorial on pyslim manual #271

Closed isadoo closed 2 years ago

isadoo commented 2 years ago

I was wondering if the tutorials for pyslim are outdated?

I'm referring to the example of using recapitate (https://tskit.dev/pyslim/docs/latest/tutorial.html#recapitation).

I run the SLiM script you provide, and then when it's time to run the python piece, it doesn't go through.

Despite following these two tutorials above word-for-word, my code fails with the following error:

Traceback (most recent call last):
  File "/Users/idoo/code_covar_evo/ANOTHERONE.py", line 12, in <module>
    ancestral_Ne=200, random_seed=5)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-packages/pyslim/methods.py", line 87, in recapitate
    return SlimTreeSequence(recap)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-packages/pyslim/slim_tree_sequence.py", line 203, in __init__
    self.individual_locations = ts.tables.individuals.location
AttributeError: can't set attribute

It seems like it is the individual_locations.setter that is missing.

After looking at the library code, TreeSequence in /tskit/trees.py/TreeSequence in particular, i discovered that there is indeed no property setter defined for [individual_locations] (https://github.com/tskit-dev/tskit/blob/fe2576d70d197f7248c14ade3710e133f8c415b5/python/tskit/trees.py#L5461-L5465). Equivalently, if i add the setter:


python3
    @individual_locations.setter
    def individual_locations(self, value):
        self.individuals_locations = value

The error changes to the following:

...
  File "/home/rxz/dev/isaslim/slimvenv/lib/python3.8/site-packages/pyslim/methods.py", line 82, in recapitate
    return SlimTreeSequence(recap)
  File "/home/rxz/dev/isaslim/slimvenv/lib/python3.8/site-packages/pyslim/slim_tree_sequence.py", line 212, in __init__
    self.individual_locations.shape = (int(len(self.individual_locations)/3), 3)
ValueError: cannot reshape array of size 24 into shape (2,3)

My questions is therefore whether the tutorial for "recapitate" outdated or is there somethign wrong with library code? Or maybe I made a mistake and I don't understand.


I have installed pyslim via pip install pyslim. My SLiM version is 3.7.1 I'm pretty sure that the .tree frile isn't the problem. If i print it on entry into the `SlimTreeSequence.initi" it does return the following table: (notice that the number of individuals in this is very small just because on this time around I changed the number of individuals from 1000 to 10)

ts is : ╔═════════════════════════╗
║TreeSequence             ║
╠═══════════════╤═════════╣
║Trees          │      946║
╟───────────────┼─────────╢
║Sequence Length│100000000║
╟───────────────┼─────────╢
║Time Units     │    ticks║
╟───────────────┼─────────╢
║Sample Nodes   │       16║
╟───────────────┼─────────╢
║Total Size     │125.9 KiB║
╚═══════════════╧═════════╝
╔═══════════╤════╤════════╤════════════╗
║Table      │Rows│Size    │Has Metadata║
╠═══════════╪════╪════════╪════════════╣
║Edges      │2413│75.4 KiB│          No║
╟───────────┼────┼────────┼────────────╢
║Individuals│   8│ 2.6 KiB│         Yes║
╟───────────┼────┼────────┼────────────╢
║Migrations │   0│ 8 Bytes│          No║
╟───────────┼────┼────────┼────────────╢
║Mutations  │   0│ 1.2 KiB│          No║
╟───────────┼────┼────────┼────────────╢
║Nodes      │ 685│19.7 KiB│         Yes║
╟───────────┼────┼────────┼────────────╢
║Populations│   3│ 2.4 KiB│         Yes║
╟───────────┼────┼────────┼────────────╢
║Provenances│   2│ 4.0 KiB│          No║
╟───────────┼────┼────────┼────────────╢
║Sites      │   0│16 Bytes│          No║
╚═══════════╧════╧════════╧════════════

Thank you!!

isadoo commented 2 years ago

I think the problem was that the current (or at least what I thought was the current) version of slim_tree_sequence.py is missing some function definitions. I found on an earlier submission (pyslim/methods.py) by Prof. Peter Ralph(https://github.com/petrelharp) which have the def of functions individual_locations, individual_ages, individual_times and individual_populations and I added that to the slim_tree_sequence.py and now it works like the tutorial expects it to work!

petrelharp commented 2 years ago

Hi, thanks, @isadoo - the problem here is that .individual_locations got moved over to tskit, so this is an error you get if you use the new tskit and the current pyslim. The fix is either to use the previous release of tskit (e.g. pip install tskit==0.5.0) or the beta version of pyslim (pip install pyslim==1.0b1). The latter is preferred, although that pyslim is designed to work with SLiM v4, so you'll get a warning message when loading older tree sequences. (the warning can safely be ignored, though)

This is probably something that's affecting other people, but I'm not sure what to do about it, besides releasing the next pyslim as soon as possible.

isadoo commented 2 years ago

Thanks, @petrelharp ! Yes, I ended up doing something similar to what you suggested (link to what I followed to anyone else reading this: https://tskit.dev/pyslim/docs/stable/development.html#sec-development). And for the tree sequences, the fact that they were a different version was giving me a little bit more trouble than just the warning so I used the pyslim.update( ) function, and it worked!

jaam92 commented 2 years ago

Hi @petrelharp

I think I am having similar issues making it through the stdpopsim tutorial but I have not figured out a work around. After following this thread, I am using the new tskit and switched to beta version of pyslim.

I am trying to run the following code which errors out on ts line

import stdpopsim
species = stdpopsim.get_species("HomSap")
contig = species.get_contig("chr22", length_multiplier=0.1)
model = species.get_demographic_model("Africa_1T12")
samples = model.get_samples(200)
engine = stdpopsim.get_engine("slim")
ts = engine.simulate(model, contig, samples, slim_scaling_factor=10)
print(ts.num_mutations)

Here is the error message:

File "/Users/Jazlyn/opt/anaconda3/envs/ibdsims/lib/python3.8/site-packages/stdpopsim/slim_engine.py", line 724, in simulate
ts = pyslim.load(ts_file.name)
File "/Users/Jazlyn/.local/lib/python3.8/site-packages/pyslim-1.0b1-py3.8.egg/pyslim/slim_tree_sequence.py", line 41, in load
raise RuntimeError("This method has been removed: use tskit.load( ) instead.")
RuntimeError: This method has been removed: use tskit.load( ) instead.

I am wondering if there is library code I can modify or if there is a work around that I am missing? I have tried modifying the slim_engine.py script but then the fix broke a bunch of later functions which I was hoping would not happen but of course did :/

Thanks for the help, Jazlyn

P.S. A couple of us also tried out the previous version of tskit and beta of pyslim to no avail

petrelharp commented 2 years ago

Hi, Jazlyn, and thanks for the report. Hmph - we generally try hard to keep everything backwards-compatible, but this next pyslim release isn't be doing that, to erase some previous bad design decisions. Apologies.

Ok - the issue here seems to be that the release of stdpopsim uses the previous release of pyslim, but the release of pyslim is not compatible with the current tskit release. The beta release of pyslim fixed compatibility with tskit, but not stdpopsim. We're aiming to have a stdpopsim release in the next month or so, which will fix things; this is a good reason to push a beta release sooner. I think your options are:

I won't be changing anything in pyslim before releasing, so you're safe using that beta. Perhaps you want to wait for the stdpopsim beta release? The code you have above will run unchanged (and inded runs fine now on github main/).

Again - many apologies, and ping if you run into more issues?

andrewkern commented 2 years ago

another option-- rather than use the current stdpopsim release that has this conflict, you can use the latest, development version which works fine.

you should be able to install it using pip install git+https://github.com/popsim-consortium/stdpopsim.git

jaam92 commented 2 years ago

Thanks @petrelharp and @andrewkern!

Definitely will go for using the github stdpopsim, beta pyslim, and current tskit seems like the best route to take here.

I appreciate the help, Jazlyn

petrelharp commented 2 years ago

We'll be getting out the new pyslim release in the next few days (which goes along with SLiM v4).