MapEntries IR; use in annotate. #3176
Conversation
catoverdrive
force-pushed
the
select-entries
branch
from
e892619
to
ed4b6d5
Compare
python/hail/tests/utils.py
Outdated
| @@ -4,7 +4,7 @@ | |||
|
|
|||
|
|
|||
| def startTestHailContext(): | |||
| hail.init(master='local[2]', min_block_size=0, quiet=True) | |||
| hail.init(master='local[2]', min_block_size=0) | |||
| .bind("va", child.typ.rvRowType) | ||
| .bind("sa", TArray(child.typ.colType)) | ||
| ) | ||
| child.typ.copy(rvRowType=newRow.typ) |
There was a problem hiding this comment.
style - should have spaces
| val globals = rvb.end() | ||
|
|
||
| rvb.start(localColsType) | ||
| rvb.addAnnotation(localColsType, colValuesBc.value) |
There was a problem hiding this comment.
prefer addUnsafeArray here
There was a problem hiding this comment.
and :( this is going to be so slow, the off-heap changes can't come fast enough!
There was a problem hiding this comment.
I'm having a bit of trouble reasoning about this. rv.region is usually the same region for every record, right? Is this going to mean that the sample annotations are added to the region once for every record in a partition?
There was a problem hiding this comment.
:( yes. But we can't get around that right now since we're not creating the original region.
There was a problem hiding this comment.
this might be untenable, even in the short term. Let's discuss at checkin
There was a problem hiding this comment.
re: addUnsafeArray---I kind of prefer the consistency of addAnnotation. It calls addUnsafeArray in the UnsafeIndexedSeq case anyways.
catoverdrive
force-pushed
the
select-entries
branch
from
00cd1d4
to
5faa2ea
Compare
catoverdrive
dismissed
tpoterba’s stale review
addressed comments and disabled IR path for now.
catoverdrive
force-pushed
the
select-entries
branch
from
79530fd
to
3b37894
Compare
* wip * add MapEntries, use in annotate_entries * fix test * fix * fix and disable * use IR for smol col annotations
* wip * add MapEntries, use in annotate_entries * fix test * fix * fix and disable * use IR for smol col annotations
* wip * add MapEntries, use in annotate_entries * fix test * fix * fix and disable * use IR for smol col annotations
I'm going to merge select/drop/annotate on the scala side in a separate PR. For now annotateEntriesExpr uses the IR path if possible for testing.