feat: Migrate to TensorFlow v2.0 #541

matthewfeickert · 2019-09-04T05:50:26Z

Description

Resolves #539

Instead of adding conditional control flows through PR #535 and PR #537 in support of PR #534 , PR #534 will be delayed in favor of migrating to TensorFlow v2.0, where eager evaluation will avoid the issues that PR #535 and PR #537 addressed.

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Migrate the TensorFlow backend and optimizer to TensorFlow v2.0 / deprecate TensorFlow v1.0 behavior
   - Changes evaluation from lazy (v1.0) to eager (v2.0 default)
* Require TensorFlow and TensorFlow Probability releases compatible with TensorFlow v2.0
* Simplify test configuration with deprecation of TensorFlow v1.0 Sessions

matthewfeickert · 2019-09-04T05:56:48Z

TODO:

Rewrite opt_tflow.py using tf.function instead of tf.compat.v1.Session
- This is a bit confusing for me at the moment to be honest.

References:

RFC TF 2.0: Functions, not Sessions

matthewfeickert · 2019-09-05T18:04:03Z

Until tf.hessians gets updated in TF 2.0 this PR is blocked as a results of gradients. So this will need to wait for a new release candidate.

matthewfeickert · 2019-11-04T05:39:40Z

Given the decision to revert the addition of "improvement" in commitizen/conventional-commit-types this was renamed to a feature.

codecov · 2020-01-11T22:01:11Z

Codecov Report

Merging #541 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #541     +/-   ##
=========================================
+ Coverage   94.86%   94.96%   +0.1%     
=========================================
  Files          52       52             
  Lines        2763     2742     -21     
  Branches      392      385      -7     
=========================================
- Hits         2621     2604     -17     
+ Misses         97       95      -2     
+ Partials       45       43      -2

Flag	Coverage Δ
#unittests	`94.96% <100%> (+0.1%)`	⬆️

Impacted Files	Coverage Δ
src/pyhf/optimize/opt_tflow.py	`100% <100%> (ø)`	⬆️
src/pyhf/tensor/tensorflow_backend.py	`96.39% <100%> (+2.95%)`	⬆️
src/pyhf/cli/infer.py	`97.67% <100%> (-0.06%)`	⬇️
src/pyhf/__init__.py	`97.72% <100%> (-0.19%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dc4d36...e670e01. Read the comment docs.

matthewfeickert · 2020-01-11T22:08:56Z

The coverage needs to get fixed here

https://codecov.io/gh/scikit-hep/pyhf/pull/541/diff?src=pr&el=tree#D3-102

but other than that this looks like it is almost ready for review.

github-actions · 2020-01-12T00:31:31Z

I've queued this up. When it gets merged, I'll create a patch release from v0.3.2 → v0.3.3 which includes the following 5 change(s) [including this PR]:

feat: Migrate to TensorFlow v2.0

refactor: Use multistage Docker build to include only built libraries in Docker image

fix: Update nbformat to v4.4 for all tested notebooks

docs: Directly link to GitHub project page from docs website

docs: Apply pydocstyle to Workspace

If you make any more changes, you probably want to re-trigger me again by removing the bumpversion/patch label and then adding it back again.

Use tf.where from TF 2.0 API

Need to add tf.errors.InvalidArgumentError to errors caused by invalid shape addition

These checks were meant for managing the session's graph, but with Sessions being a legacy component of TF v1.0 there is no need and can use the same 'tolist' as NumPy and PyTorch

matthewfeickert · 2020-01-12T19:05:22Z

just to understand better: this does not enable eager execution, but just removes all the session stuff and handles gradients differently?

No this does enable eager execution. If you check out the docstrings that I added that's what we'll be getting from now on.

If we want to move to lazy eval then we'll need to revise this.

lukasheinrich · 2020-01-12T19:10:38Z

ah right, sorry I overlooked this

matthewfeickert · 2020-01-12T19:12:44Z

ah right, sorry I overlooked this

No I should have mentioned this in the PR body. I've now updated the commit history message for it.

kratsg · 2020-01-12T19:36:20Z

setup.py

+    'tensorflow': [
+        'tensorflow~=2.0',
+        'tensorflow-probability~=0.8',
+        'numpy<2.0,>=1.16.0',


do we need to keep this around given: https://github.com/tensorflow/tensorflow/blob/5364121e858be715439c328c5f67de8441050f4b/tensorflow/tools/pip_package/setup.py#L61

Yeah, we can ~~simplify that to just numpy~=1.16~~ drop that totally. Good point!

Triggered by #541 via GitHub Actions.

matthewfeickert added the feat/enhancement New feature or request label Sep 4, 2019

matthewfeickert self-assigned this Sep 4, 2019

This was referenced Sep 4, 2019

feat: Add 'conditional' method to tensorlib API #535

Merged

Use conditional control flow in 'pvals_from_teststat' #537

Closed

This was referenced Sep 6, 2019

feat: Add backend option to pyhf CLI #534

Merged

pyhf 2019 into 2020 Roadmap #561

Open

matthewfeickert force-pushed the revise/migrate-to-TF-2.0 branch from b15ec30 to 5fef166 Compare October 1, 2019 07:48

matthewfeickert changed the title ~~Migrate to TensorFlow v2.0~~ improvement: Migrate to TensorFlow v2.0 Oct 1, 2019

matthewfeickert force-pushed the revise/migrate-to-TF-2.0 branch from cfca435 to f753c27 Compare October 2, 2019 07:54

matthewfeickert mentioned this pull request Oct 6, 2019

Discrepancy between listed semantic PR titles and those approved by PR bot zeke/semantic-pull-requests#72

Closed

matthewfeickert changed the title ~~improvement: Migrate to TensorFlow v2.0~~ feat: Migrate to TensorFlow v2.0 Nov 4, 2019

matthewfeickert force-pushed the master branch from 097c4bf to 40d8243 Compare December 18, 2019 15:34

matthewfeickert force-pushed the revise/migrate-to-TF-2.0 branch from f753c27 to 24a8295 Compare December 30, 2019 06:48

matthewfeickert force-pushed the revise/migrate-to-TF-2.0 branch from 24a8295 to 33d8bc2 Compare January 11, 2020 20:54

matthewfeickert added bumpversion/patch Create a patch version release API Changes the public API docs Documentation related labels Jan 11, 2020

scikit-hep deleted a comment from github-actions bot Jan 12, 2020

matthewfeickert marked this pull request as ready for review January 12, 2020 00:47

matthewfeickert requested review from kratsg and lukasheinrich January 12, 2020 00:47

matthewfeickert mentioned this pull request Jan 12, 2020

fix: Correct iminuit optimizer verbose setting typo #724

Merged

4 tasks

matthewfeickert added the tests pytest label Jan 12, 2020

matthewfeickert added 20 commits January 12, 2020 13:02

Migrate 'simple_broadcast' docstring to TF 2.0

6f6c6e6

Migrate 'poisson_logpdf' docstring to TF 2.0

83d6e16

Migrate 'poisson' docstring to TF 2.0

ef65300

Migrate 'normal_logpdf' docstring to TF 2.0

7012e1b

Migrate 'normal' docstring to TF 2.0

98b792a

Migrate 'normal_cdf' docstring to TF 2.0

8763880

Use a test for tensor that works in eager eval

f50d7de

Migrate 'where' to TF 2.0

fc32c7a

Use tf.where from TF 2.0 API

Remove local variable for pyflakes

6aedf16

Deprecate TF v1.0 Sessions in tests

0f21594

Upgrade to TensorFlow 2.0

4351616

Migrate 'poisson_dist' and 'normal_dist' docstrings to TF 2.0

df02ae9

Use TensorFlow Probability v0.8 compatible releases

5fc8be7

Remove TF v1.0 sessions from CLI infer

7168841

Migrate 'conditional' docstring to TF 2.0

99a1c7f

Migrate test_tensor.py to TF 2.0

d05c3b7

Need to add tf.errors.InvalidArgumentError to errors caused by invalid shape addition

Use GradientTape for TF optimizer and pass test_optim.py

4686757

Update ROADMAP

ab131c1

Remove legacy checks

6928460

These checks were meant for managing the session's graph, but with Sessions being a legacy component of TF v1.0 there is no need and can use the same 'tolist' as NumPy and PyTorch

Removed unused TF imports to pass pyflakes

ad5ba15

matthewfeickert force-pushed the revise/migrate-to-TF-2.0 branch from fc1fcb5 to ad5ba15 Compare January 12, 2020 19:04

kratsg approved these changes Jan 12, 2020

View reviewed changes

Simplify tensorflow extra requires

e670e01

matthewfeickert merged commit ebf7ece into master Jan 12, 2020

matthewfeickert deleted the revise/migrate-to-TF-2.0 branch January 12, 2020 20:34

kratsg pushed a commit that referenced this pull request Jan 12, 2020

Bump version: 0.3.2 → 0.3.3

98b2cb0

Triggered by #541 via GitHub Actions.

matthewfeickert mentioned this pull request Jan 12, 2020

Investigate using TensorFlow v2.0 with lazy evaluation #727

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Migrate to TensorFlow v2.0 #541

feat: Migrate to TensorFlow v2.0 #541

Uh oh!

matthewfeickert commented Sep 4, 2019 •

edited

Loading

Uh oh!

matthewfeickert commented Sep 4, 2019 •

edited

Loading

Uh oh!

matthewfeickert commented Sep 5, 2019

Uh oh!

matthewfeickert commented Nov 4, 2019 •

edited

Loading

Uh oh!

codecov bot commented Jan 11, 2020 •

edited

Loading

Uh oh!

matthewfeickert commented Jan 11, 2020

Uh oh!

github-actions bot commented Jan 12, 2020

Uh oh!

matthewfeickert commented Jan 12, 2020 •

edited

Loading

Uh oh!

lukasheinrich commented Jan 12, 2020

Uh oh!

matthewfeickert commented Jan 12, 2020

Uh oh!

kratsg Jan 12, 2020

Uh oh!

matthewfeickert Jan 12, 2020 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: Migrate to TensorFlow v2.0 #541

feat: Migrate to TensorFlow v2.0 #541

Uh oh!

Conversation

matthewfeickert commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist Before Requesting Reviewer

Before Merging

Uh oh!

matthewfeickert commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthewfeickert commented Sep 5, 2019

Uh oh!

matthewfeickert commented Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matthewfeickert commented Jan 11, 2020

Uh oh!

github-actions bot commented Jan 12, 2020

Uh oh!

matthewfeickert commented Jan 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukasheinrich commented Jan 12, 2020

Uh oh!

matthewfeickert commented Jan 12, 2020

Uh oh!

kratsg Jan 12, 2020

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Jan 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matthewfeickert commented Sep 4, 2019 •

edited

Loading

matthewfeickert commented Sep 4, 2019 •

edited

Loading

matthewfeickert commented Nov 4, 2019 •

edited

Loading

codecov bot commented Jan 11, 2020 •

edited

Loading

matthewfeickert commented Jan 12, 2020 •

edited

Loading

matthewfeickert Jan 12, 2020 •

edited

Loading