Skip to content
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

Refactored tensor equal assertion test into common utility #39

Closed
wants to merge 4 commits into from

Conversation

RdoubleA
Copy link
Contributor

@RdoubleA RdoubleA commented May 9, 2022

Summary:
The use of torch.testing.assert_close(actual, expected) was a prevalent pattern throughout the codebase, each having to create its own assertion failure message. This was consolidated into a common utility function in test/test_utils.py such that this assertion is only defined in a single location and all current/future unit tests simply have to import this to test if two tensors are equal without having to write the same assertion failure message.

Test plan:
Since this impacts potentially all unit tests, I ran the entire test suite with pytest -vv.

@RdoubleA RdoubleA requested a review from langong347 May 9, 2022 22:46
@RdoubleA RdoubleA self-assigned this May 9, 2022
@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 May 9, 2022
Copy link
Contributor

@langong347 langong347 left a comment

Choose a reason for hiding this comment

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

Also please update our internal only unit test (under fb/). Can be a separate diff.

test/test_utils.py Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@RdoubleA has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

suiyoubi added a commit to suiyoubi/multimodal that referenced this pull request May 11, 2022
* Change ImageDataModule to the old style such as we don't need to extract it. Fixed minor bugs.

* add lightning logs to gitignore

* change config file for imagenet directory

* add transformer denpendency

* add SAVE_DIR

* Add default for imagent_tar

* Refactored tensor equal assertion test into common utility (facebookresearch#39)

Summary:
The use of `torch.testing.assert_close(actual, expected)` was a prevalent pattern throughout the codebase, each having to create its own assertion failure message. This was consolidated into a common utility function in `test/test_utils.py` such that this assertion is only defined in a single location and all current/future unit tests simply have to import this to test if two tensors are equal without having to write the same assertion failure message.

Pull Request resolved: facebookresearch#39

Test Plan: Since this impacts potentially all unit tests, I ran the entire test suite with `pytest -vv`.

Reviewed By: langong347

Differential Revision: D36301056

Pulled By: RdoubleA

fbshipit-source-id: 0e0f62717186efa1307f545ac1bbe82ffed65a58

* quick fix for zero-shot validation

* Add num_nodes in training conf

Co-authored-by: aot <aot@nvidia.com>
Co-authored-by: Rafi Ayub <rafiayub@fb.com>
@ankitade ankitade deleted the test_utils branch December 7, 2022 18:46
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants