Skip to content

Commit

Permalink
[Bugfix][Performance] Fix Column to copy 'self.index' to location of …
Browse files Browse the repository at this point in the history
…'self.storage' only on read, and use kwargs. (dmlc#2267)

* Pass framework arguments to copy of frames

* Copy for subframe

* Fix mismatched contexts

Co-authored-by: Quan (Andy) Gan <[email protected]>
  • Loading branch information
nv-dlasalle and BarclayII committed Nov 27, 2020
1 parent d1add2e commit 24d25d0
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions python/dgl/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ def data(self):
# copy index to the same context of storage.
# Copy index is usually cheaper than copy data
if F.context(self.storage) != F.context(self.index):
self.index = F.copy_to(self.index, F.context(self.storage))
kwargs = {}
if self.device is not None:
kwargs = self.device[1]
self.index = F.copy_to(self.index, F.context(self.storage), **kwargs)
self.storage = F.gather_row(self.storage, self.index)
self.index = None

Expand Down Expand Up @@ -148,8 +151,6 @@ def to(self, device, **kwargs): # pylint: disable=invalid-name
"""
col = self.clone()
col.device = (device, kwargs)
if self.index is not None:
col.index = F.copy_to(self.index, device)
return col

def __getitem__(self, rowids):
Expand Down Expand Up @@ -253,6 +254,12 @@ def subcolumn(self, rowids):
if self.index is None:
return Column(self.storage, self.scheme, rowids, self.device)
else:
if F.context(self.index) != F.context(rowids):
# make sure index and row ids are on the same context
kwargs = {}
if self.device is not None:
kwargs = self.device[1]
rowids = F.copy_to(rowids, F.context(self.index), **kwargs)
return Column(self.storage, self.scheme, F.gather_row(self.index, rowids), self.device)

@staticmethod
Expand Down

0 comments on commit 24d25d0

Please sign in to comment.