diff --git a/governance/api-reviews.md b/governance/api-reviews.md index 61dc3e64e..78106b2c9 100644 --- a/governance/api-reviews.md +++ b/governance/api-reviews.md @@ -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 +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 +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 + 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).