cmap / cmapPy

Assorted tools for interacting with .gct, .gctx files and other Connectivity Map (Broad Institute) data/tools
https://clue.io/cmapPy/index.html
BSD 3-Clause "New" or "Revised" License
124 stars 74 forks source link

Subsetting picks wrong dimension to subset over first #71

Open darintay opened 3 years ago

darintay commented 3 years ago

There is some logic in parse_data_df (pasted below) that attempts to pick the best dimension to subset over first, but it isn't quite right.

def parse_data_df(data_dset, ridx, cidx, row_meta, col_meta):
  if len(ridx) == len(row_meta.index) and len(cidx) == len(col_meta.index):  # no subset
        data_array = np.empty(data_dset.shape, dtype=np.float32)
        data_dset.read_direct(data_array)
        data_array = data_array.transpose()
  elif len(ridx) <= len(cidx):
        first_subset = data_dset[:, ridx].astype(np.float32)
        data_array = first_subset[cidx, :].transpose()
  elif len(cidx) < len(ridx):
        first_subset = data_dset[cidx, :].astype(np.float32)
        data_array = first_subset[:, ridx].transpose()

For example, imagine you're parsing a .gctx with 720216 cols and 12328 rows, and you want to pull out 20000 columns.

The subset logic is going to try to subset on rows first because 12328 < 20000. But we're not even subsetting on rows here, that is all of them. This results in the entire array getting temporarily allocated into memory.

The better heuristic is to minimize the size of the intermediate array - you want to pick the minimum of: len(ridx) len(col_meta.index) vs len(cidx) len(row_meta.index)

tnat1031 commented 3 years ago

@darintay thanks very much for the feedback. Would you be willing to generate a pull request with the proposed changes?