Skip to content

Conversation

Moh-Yakoub
Copy link
Contributor

Fixes #1469

Description:

Parameterize some metric tests.

I started with couple of test cases that had obvious use of test parameterization. Will refactor more metric tests in later PRs

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks for experimenting with parametrize @Moh-Yakoub
IMO, this is still something to think about on how to use it properly. I left my comments on the PR.

with pytest.raises(ValueError, match=r"If recall argument is provided, output_transform should be None"):
r = Recall(average=False)
Fbeta(1.0, recall=r, output_transform=lambda x: x)
@pytest.mark.parametrize(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this usage, it makes thing more cumbersome

), f"{type(loss._sum.device)}:{loss._sum.device} vs {type(metric_device)}:{metric_device}"


def test_sum_detached():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove this test ?


def test_compute_on_criterion():
loss = Loss(nn.NLLLoss())
@pytest.mark.parametrize("loss_function", [nll_loss, nn.NLLLoss()])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good usage 👍

@Moh-Yakoub Moh-Yakoub closed this Feb 28, 2021
@Moh-Yakoub Moh-Yakoub deleted the parameterize_some_metric_tests branch February 28, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parametrize metric tests
2 participants