Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graph.assert_close() shouldn't ignore pulse ordering. #328

Closed
grahamgower opened this issue Jun 13, 2021 · 2 comments · Fixed by #362
Closed

Graph.assert_close() shouldn't ignore pulse ordering. #328

grahamgower opened this issue Jun 13, 2021 · 2 comments · Fixed by #362
Labels
bug Something isn't working docs Improvements or additions to documentation
Milestone

Comments

@grahamgower
Copy link
Member

The docs currently say that the order in which admixture pulses were specified is ignored during the comparison. But this shouldn't strictly be the case. If multiple pulses are specified for the same time, then the order in which pulses were defined will matter. Hence the correct thing to do is to (stable)sort pulses by the time field and then compare.

@grahamgower grahamgower added bug Something isn't working docs Improvements or additions to documentation labels Jun 13, 2021
@grahamgower
Copy link
Member Author

Actually, it might be more tricky that this... The order of pulses with the same time value only matters when the pulses have demes in common. E.g. for pulse 1: source=A, dest=B, pulse 2: source=B, dest=C, the pulse order matters. But in other cases, e.g. pulse 1: source=A, dest=B, pulse 2: source=X, dest=Y, the order of pulses should be ignored.

@jeromekelleher
Copy link
Member

Better to be overly strict and require a bit of arbitrary sortedness than to falsely claim equality, IMO

@grahamgower grahamgower added this to the 0.1.3 milestone Jun 25, 2021
@grahamgower grahamgower modified the milestones: 0.1.3, 0.2.0 Jul 9, 2021
grahamgower added a commit to grahamgower/demes-python that referenced this issue Jul 23, 2021
@grahamgower grahamgower modified the milestones: 0.2.0, 0.1.3 Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants