Skip to content
This repository was archived by the owner on Jul 7, 2023. It is now read-only.

add a metrics for calculating pearson correlation coefficient #1274

Merged
merged 8 commits into from
Dec 5, 2018

Conversation

luffy06
Copy link
Contributor

@luffy06 luffy06 commented Dec 4, 2018

Dear community, I'm using tensor2tensor for text similarity tasks and I find no metrics for calculating pearson correlation coefficient.

@googlebot googlebot added the cla: yes PR author has signed CLA label Dec 4, 2018
@luffy06 luffy06 closed this Dec 5, 2018
@luffy06 luffy06 deleted the pearson_metrics branch December 5, 2018 01:01
@afrozenator
Copy link
Contributor

Hi @luffy06 - Thanks for the PR - I was qualifying the change internally to accept in the main branch, any reason you closed this PR?

Thanks!

@luffy06 luffy06 restored the pearson_metrics branch December 5, 2018 01:02
@luffy06
Copy link
Contributor Author

luffy06 commented Dec 5, 2018

@afrozenator I'm sorry, there are some bugs when it is building on Travis-CI. I'll fix it and make a pull request again.

@afrozenator
Copy link
Contributor

I just found out that the test is slightly broken

ValueError: Can not squeeze dim[2], expected a dimension of 1, got 4 for 'pearson_r/remove_squeezable_dimensions/Squeeze' (op: 'Squeeze') with input shapes: [12,1,4].

Please note that travis is currently broken, so if your metrics_test passes locally, you should be fine -- thanks again for this, this is very nice addtion.

@afrozenator
Copy link
Contributor

Just reopen this when you are done and then we can take it forward. Thanks again!

@luffy06 luffy06 reopened this Dec 5, 2018
@afrozenator
Copy link
Contributor

This still errors internally with tf-nightly (i.e. pip install tf-nightly instead of pip install tensorflow) with the same error as reported previously

ValueError: Can not squeeze dim[2], expected a dimension of 1, got 2 for 'pearson_r/remove_squeezable_dimensions/Squeeze' (op: 'Squeeze') with input shapes: [12,1,2].

@luffy06
Copy link
Contributor Author

luffy06 commented Dec 5, 2018

There are some errors in my metric_test method, I'm fixing it.

@luffy06 luffy06 closed this Dec 5, 2018
@luffy06 luffy06 reopened this Dec 5, 2018
@luffy06
Copy link
Contributor Author

luffy06 commented Dec 5, 2018

Hi @afrozenator, I've fixed my code errors and built success in Travis. Please qualify my PR. Thanks a lot.

@afrozenator
Copy link
Contributor

Beautiful! Many thanks @luffy06 !

@afrozenator afrozenator merged commit 3689c42 into tensorflow:master Dec 5, 2018
tensorflow-copybara pushed a commit that referenced this pull request Dec 5, 2018
PiperOrigin-RevId: 224104845
theorm added a commit to theorm/tensor2tensor that referenced this pull request Dec 5, 2018
* master:
  internal merge of PR tensorflow#1274
  add a metrics for calculating pearson correlation coefficient (tensorflow#1274)
  Rm dataset
  internal merge of PR tensorflow#1269
  Improved model free eval. (tensorflow#1269)
  Re-enable eval summaries in the correct directory
  Create an integer problem_0_steps variable. (tensorflow#1273)
  Small En-Ro dataset for low-resource experiments, use that one in translation multi-problem; and a few corrections.
  Rm evaluation_hooks summarysaver. This shouldn't be here. Directory may be eval_train. Estimator is already logging summaries for eval.
  Add support for multiproblem to mtf_model
  add explicit taskid check for all multiproblem_weight functions
  Readd nfg to open-source T2T for external collaboration.
  Starting work on a new mesh_tensorflow Transformer implementation.  This implementation lives in the mesh_tensorflow library and does not depend on Tensor2Tensor.
kpe pushed a commit to kpe/tensor2tensor that referenced this pull request Mar 2, 2019
…flow#1274)

* add pearson metrics and test method

* fix test errors

* update metrics test: dimension error

* update metrics test

* update metrics test

* update metrics test

* update metrics test

* fix errors
kpe pushed a commit to kpe/tensor2tensor that referenced this pull request Mar 2, 2019
PiperOrigin-RevId: 224104845
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants