Skip to content

[vds] Fix local_to_global to support non-ascending local alleles#12543

Merged
danking merged 3 commits intohail-is:mainfrom
tpoterba:fix-local-to-global
Dec 14, 2022
Merged

[vds] Fix local_to_global to support non-ascending local alleles#12543
danking merged 3 commits intohail-is:mainfrom
tpoterba:fix-local-to-global

Conversation

@tpoterba
Copy link
Contributor

@tpoterba tpoterba commented Dec 7, 2022

Fixes #12534

@@ -82,21 +82,18 @@ def local_to_global(array, local_alleles, n_alleles, fill_value, number):
-------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have an example that uses the fields of a VDS. Do we have a VDS in the tests we can use for that purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't have a vds in tests, nope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the slack thread, I really want to hide this behind a packaged option in to_dense_mt or something to reindex multiple fields

assert hl.eval(hl.vds.local_to_global(lad, local_alleles, 4, 0, number='R')) == [1, 9, 0, 10]
assert hl.eval(hl.vds.local_to_global(lpl, local_alleles, 4, 999, number='G')) == [1001, 1002, 1005, 999, 999, 999, 1004, 1003, 999, 0]

assert hl.eval(hl.vds.local_to_global([0, 1, 2, 3, 4, 5], [0, 2, 1], 3, 0, number='G')) == [0, 3, 5, 1, 4, 2] No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think I'm missing something. Do I have a step wrong here:

  1. 1, as a local GT triangle number, definitionally equivalent to
  2. 0/1 in local alleles which is
  3. 0/2 in global alleles (LA[1] == 2 in this example)
  4. 2 in global GT triangle number.

local genotypes triangle numbers:

0    1    2
     3    4
          5

local genotypes as local alleles:

0/0  0/1  0/2
     1/1  1/2
          2/2

local genotypes as global alleles:

0/0  0/2  0/1
     2/2  2/1
          1/1

local genotypes as global triangle numbers:

  0    2    1
       5    4
            3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The triangles go the other way:

0/0
0/1 1/1
0/2 1/2 2/2
0
1 2
3 4 5

The intuition here is that the addition of a 2 allele shouldn't affect any indices of calls involving only 0 and 1 alleles.

@danking danking merged commit 3133bf9 into hail-is:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hl.vds.local_to_global should support non-increasing local alleles

3 participants