Skip to content

Implement unittests for MemmapTensor. #231

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

Merged
merged 9 commits into from
Jun 29, 2022
Merged

Implement unittests for MemmapTensor. #231

merged 9 commits into from
Jun 29, 2022

Conversation

fredfung007
Copy link
Contributor

  • Added a test if the created MemmapTensor is on the same device as the input tensor.
  • Added a test if device is correct when .to(device) is called.
  • Added a test if the device arg for MemmapTensor init is respected.
  • Added a test if all entries are zeros when MemmapTensor is created with size.
  • Added a test if MemmapTensor can be created with a given data type and shape.

This PR closes #221.

Signed-off-by: Fred Feng <fung@fb.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 28, 2022
fredfung007 and others added 2 commits June 28, 2022 12:38
Signed-off-by: Fred Feng <fung@fb.com>
Copy link
Collaborator

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

Fantastic work, thanks so much for this! Almost good to go!

@vmoens vmoens added the Tests Incomplete or broken unit tests label Jun 28, 2022
Signed-off-by: Fred Feng <fung@fb.com>
@fredfung007 fredfung007 changed the title Implement unittests for MemmapTensor. [WIP] Implement unittests for MemmapTensor. Jun 28, 2022
Signed-off-by: Fred Feng <fung@fb.com>
Signed-off-by: Fred Feng <fung@fb.com>
"""
value.to(device)
expected_memmap_tensor = MemmapTensor(value)
m1 = MemmapTensor([3, 4])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guss this guy should be created on device too

    m1 = MemmapTensor([3, 4], device=device)

assert m1.shape == (3, 4)
assert torch.all(m1 == expected_memmap_tensor)
assert torch.all(m1 + torch.ones([3, 4], device=device) == 1)
m2 = MemmapTensor(3, 4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here
by the way, we could perhaps nest those two tests using parameters

@pytest.mark.parametrize("shape", [ [[3, 4]], [3, 4] ])
def test_foo(shape, device):
    m1 = MemmapTensor(*shape, device=device)  # first cast is a list, second case is a sequence of integers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @vmoens!
I have made corresponding changes. I was setting up the WSL environment locally on a windows machine to test cuda/cpu. Sorry for delayed responses.

@fredfung007 fredfung007 changed the title [WIP] Implement unittests for MemmapTensor. Implement unittests for MemmapTensor. Jun 29, 2022
Copy link
Collaborator

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

Splendid! Thanks so much for this!

@vmoens vmoens merged commit 5833b2f into pytorch:main Jun 29, 2022
@fredfung007 fredfung007 deleted the test/memmap_test branch June 29, 2022 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Tests Incomplete or broken unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test MemmapTensor creation with shape, dtype and device
3 participants