-
Notifications
You must be signed in to change notification settings - Fork 62
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
Correct Shape of scdl output matirx #708
base: main
Are you sure you want to change the base?
Conversation
@@ -27,14 +29,19 @@ def collate_sparse_matrix_batch(batch: list[torch.Tensor]) -> torch.Tensor: | |||
Returns: | |||
The tensors collated into a CSR (Compressed Sparse Row) Format. | |||
""" | |||
n_features = batch[0][1][0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment explaining the hardcoded indexes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment
@@ -240,6 +240,7 @@ def __init__( | |||
paginated_load_cutoff: int = 10_000, | |||
load_block_row_size: int = 1_000_000, | |||
feature_index_name="feature_id", | |||
return_padded: bool = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level, how does the code know what the pad size is? Or max size to pad to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses the return get_row_padded
for __getitem__
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the docstring?
Signed-off-by: Intron7 <[email protected]>
sub-packages/bionemo-scdl/src/bionemo/scdl/util/torch_dataloader_utils.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Severin! Could you please add a unit test for the new feature and update docstrings?
…er_utils.py Co-authored-by: Steven Kothen-Hill <[email protected]> Signed-off-by: Severin Dicks <[email protected]>
I'll add a test tomorrow morning |
Signed-off-by: Intron7 <[email protected]>
Signed-off-by: Intron7 <[email protected]>
Signed-off-by: Intron7 <[email protected]>
Signed-off-by: Intron7 <[email protected]>
Signed-off-by: Intron7 <[email protected]>
updated the tests to work with the returned tuple. Also I added 2 new tests for return padded and the shape |
I also updated the |
This fixes the size of the output matrix for scdl.
It also enables users to return a dense array