-
Notifications
You must be signed in to change notification settings - Fork 577
experimental API policy #218
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cafd747
experimental API policy
alextp 8165cb9
clarifying known deficiency
alextp edb0292
Update governance/api-reviews.md
alextp a75ea94
Update governance/api-reviews.md
alextp 3453e27
Merge branch 'experimental' of github.com:alextp/community into exper…
alextp 935e14d
clarifications
alextp bf57a9d
clarify policy for stuff which keeps changing
alextp de161b6
reference experimental section from other places
alextp 8fb29b0
clarify addons
alextp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,8 +24,8 @@ We avoid backwards-incompatible API changes. We also avoid | |
backwards-incompatible behavior changes, such as restricting the set of valid | ||
inputs to a function or extending the set of valid outputs of a function. Adding | ||
support for previously not supported behavior is okay, as are changes to | ||
explicitly experimental APIs (within reason). When needing to provide a new or | ||
different behavior, we strongly prefer a new version of the API over breaking | ||
explicitly experimental APIs (see section below). When needing to provide a new | ||
or different behavior, we strongly prefer a new version of the API over breaking | ||
backwards compatibility. Note that we are free to deprecate APIs; we just cannot | ||
break code which relies on their documented behavior. We need to worry about | ||
backward compatibility both of our python APIs and of the serialized GraphDefs, | ||
|
@@ -37,15 +37,6 @@ produced by currently correct python code without a three weeks notice. This | |
comes up most frequently when adding new ops, but also applies to non-obvious | ||
things such as the graph emitted by gradients or pfor. | ||
|
||
Including the name “experimental” in an API endpoint allows you to break | ||
backwards compatibility in the future, as noted above. However, we still prefer | ||
to mark the experimental API as deprecated for one release before removing it in | ||
the subsequent release. Please do not use the experimental namespace as an | ||
escape hatch for bad code or functionality you | ||
don’t-really-want-but-need-for-now; experimental APIs should be APIs that we | ||
intend to make standard in the near future, but have some lingering questions to | ||
resolve first. | ||
|
||
|
||
### Docstrings | ||
|
||
|
@@ -89,7 +80,7 @@ the next. We would like new experimental symbols to be things which will | |
eventually end up in core TF as opposed to things we expect will be phased out | ||
with no clear replacement. The best expectation to have for an experimental | ||
endpoint is that the “experimental” will simply be removed. If you don’t believe | ||
that’ll work, it should probably not be added in its current form. | ||
that’ll work, it should probably not be added in its current form. | ||
|
||
### Style | ||
|
||
|
@@ -200,3 +191,68 @@ When adding new ops, look for: | |
it CPU, GPU, XLA, TPU, etc) and prefer to have at least a plan for writing | ||
device-agnostic code | ||
- should the python layer for this operation support raggedtensor/sparsetensor? | ||
|
||
## Experimental APIs | ||
|
||
Experimental APIs are APIs which have the word 'experimental' somewhere in their | ||
name; for example `tf.experimental.foo`, or `tf.foo.experimental.Bar`, or | ||
`tf.foo(experimental_bar=True)` or `tf.Foo().experimental_bar()`. We generally | ||
prefer experimental namespaces when possible, so prefer | ||
`tf.foo.experimental.Bar` over `tf.foo.ExperimentalBar`. | ||
|
||
Experimental APIs are APIs intended to be added to TensorFlow as-is, but which | ||
alextp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
we reserve the right to change in backwards-incompatible ways if we have | ||
to. This is different from apis in `tensorflow/addons`, many of which are not | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this makes it more clear - thanks! |
||
necessarily intended to be added to core TF as they might have a more narrow use | ||
case initially (if APIs in `tensorflow/addons` do become widely useful they can | ||
"graduate" to core, either using experimental or not). | ||
|
||
No temporary APIs should be added to experimental (i.e. "we just need this until | ||
certain bugfix or certain new feature becomes available" is not a valid reason | ||
to add an API with experimental in the name.) | ||
|
||
No API with known deficiencies should be added to experimental. Experimental | ||
APIs should, to the best of our knowledge, not be expected to change in a known | ||
way (no argument with a known bad name, etc). Experimental can, however, be used | ||
for APIs which are a work-in-progress: it's fine to add experimental methods to | ||
a base class even if those methods are only implemented on some subclasses as | ||
long as we expect all classes to eventually implement those. | ||
|
||
The same amount of due diligence required for a real API is required for an | ||
experimental API: this means tests, benchmarks, documentation, end-to-end | ||
examples, etc | ||
|
||
Experimental APIs are not a license to break users. This means: | ||
1. we do not remove experimental APIs which are widely used without an effort | ||
to help migrate users away | ||
2. experimental APIs are not removed without warning and don't have | ||
alextp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
backwards-incompatible changes made to them without warning (the warning can be | ||
a deprecation on version 2.x and removal on 2.x+1, but plain removal on 2.x | ||
with no notice on 2.x-1 is not ok) | ||
|
||
Small changes which are mentioned in relnotes and have obvious fixes might be | ||
made (for example if adding a new argument to a long argument list and we | ||
believe there are few pass-by-position users we might allow the new argument to | ||
be added to the middle and not the end of the parameter list). | ||
|
||
Large backwards-incompatible changes to experimental APIs still require an | ||
`experimental_foo_v2` or similar backwards-compatible evolution plan to avoid | ||
breaking users of the existing experimental API. | ||
|
||
No API endpoint should stay in experimental forever. If a particular | ||
experimental API hasn't had major changes in two minor releases we should remove | ||
the experimental annotation from the API name or delete it. If we do want to | ||
delete it we need to have a deprecation plan that can migrate all users to some | ||
other API endpoint or composition of existing APIs. In rare cases experimental | ||
APIs can continue to be iterated on after many releases (see TPUStrategy); this | ||
only applies for fairly large API surfaces. | ||
|
||
When removing the experimental annotation we should, if at all possible, allow | ||
escape routes to not break existing code. This means toplevel symbols | ||
`tf.experimental.foo` and methods like `tf.Class.experimental_foo` should get a | ||
deprecation warning on 2.x before deletion on 2.x+1; we should use the | ||
doc_controls decorators to not pollute API docs with deprecated "graduated" | ||
experimental APIs. For experimental function arguments we should consider | ||
catching `**kwargs` to raise the proper warnings for at least one version (note | ||
though that `**kwargs` is generally discouraged from our APIs; we prefer | ||
explicitly named keyword arguments if at all possible). |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.