-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix pearson correlation.py #3101
Fix pearson correlation.py #3101
Conversation
Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the denominator is assigned a value of 1.
Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the denominator is assigned a value of 1.
Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the pearson_r is assigned a value of 0.
Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the pearson_r is assigned a value of 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.
Thanks for the bug fix! Does the test you added fail without the changes that you made?
There are some minor pylint things to fix (you can see them here: http://build.allennlp.org/viewLog.html?buildId=18000&buildTypeId=AllenNLP_AllenNLPPullRequests&tab=buildLog). After that, this should be good to merge.
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.
The reason for fixing this bug is that in some input data, a batch's predictions or labels may be exactly the same. Then when the pearson_correlation.py is called, it will appear that the predictions_variance or labels_variance of the batch is 0 when calculating the variance, resulting in:
ZeroDivisionError: float division by zero
The two tests are separated because: in most cases, the data is constructed like predictions_1; but in a few cases, for example: predictions_2, the data of such a batch is exactly the same.
@matt-gardner please review |
@matt-gardner can you take another look at this? |
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, LGTM! Sorry to be super slow with this, I've been traveling for the last few weeks.
* fix bug: ZeroDivisionError: float division by zero Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the denominator is assigned a value of 1. * fix bug: ZeroDivisionError: float division by zero Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the denominator is assigned a value of 1. * fix bug: ZeroDivisionError: float division by zero Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the pearson_r is assigned a value of 0. * fix bug: ZeroDivisionError: float division by zero Since the input tensor may be, for example, a tensor ([[0.,0.,0.,0.], [0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the pearson_r is assigned a value of 0. * fix some pylint things fix some pylint things * Update pearson_correlation.py * Update pearson_correlation_test.py * Update pearson_correlation_test.py
Fixes #3102. Since the input tensor may be, for example(a batch of label), a tensor ([[0.,0.,0.,0.],
[0.,0.,0.,0. ]), there will be a case where (math.sqrt(predictions_variance) or math.sqrt(labels_variance)) is zero, so a judgment is added here to prevent the denominator from being zero. If it is zero, the denominator is assigned a value of 1.