search

google-ai-edge / ai-edge-torch

Supporting PyTorch models with the Google AI Edge TFLite runtime.
Apache License 2.0
228 stars 26 forks source link

Unpin tf-nightly in requirements.txt #27

Closed chunnienc closed 1 month ago

chunnienc commented 1 month ago

ai-edge-torch-nightly will always pull in the latest tf-nightly. This will allow bug fixes and improvements to the TFLite Converter to be automatically available for ai-edge-torch without the need to manually bump up the tf-nightly version. (from @advaitjain)

This PR also disables pip cache in CIs (python unit tests and model coverage) to always collect latest tf-nightly. If regression happens due to tf-nightly, the failures would be captured in nightly readme badges and PR pre merge checks.

BUG=none

chunnienc commented 1 month ago
  • Note that the build badge will also reflect this state -- which build badge will show this? unit tests?

This part is a bit complicated. We have the pip cache enabled for CIs so it won't always use the latest tf-nightly for testing. Only when the soft pin (>=ver) is updated the cache would guarantee to be invalidated.

This PR somehow assumes that ai-edge-torch almost never fails with latest tf-nightly. If a regression happens with latest tf-nightly, we will then need to update the soft pin back to a == hard pin until the fixes are landed in tf-nightly. For such case I'd rather have == in requirements.txt and encourage updating it more frequently (or setup CIs to do so with model coverage and unit tests).

advaitjain commented 1 month ago
  • Note that the build badge will also reflect this state -- which build badge will show this? unit tests?

This part is a bit complicated. We have the pip cache enabled for CIs so it won't always use the latest tf-nightly for testing. Only when the soft pin (>=ver) is updated the cache would guarantee to be invalidated.

This PR somehow assumes that ai-edge-torch almost never fails with latest tf-nightly. If a regression happens with latest tf-nightly, we will then need to update the soft pin back to a == hard pin until the fixes are landed in tf-nightly. For such case I'd rather have == in requirements.txt and encourage updating it more frequently (or setup CIs to do so with model coverage and unit tests).

Understood and I'm also comfortable with this assumption for now.

What I would still like to have is a build badge that shows when this breakage happens so that we can avoid a situation where folks see errors locally without a way to verify if our CI is also showing a regression. Is there something we can do on this front?

chunnienc commented 1 month ago

What I would still like to have is a build badge that shows when this breakage happens so that we can avoid a situation where folks see errors locally without a way to verify if our CI is also showing a regression. Is there something we can do on this front?

Minimal: disable pip cache for all nightly CIs to use latest tf-nightly, where the status are shown in readme badges.

Full: disable pip cache in all CIs. PRs may fail surprisingly if tf-nightly regression happens. (Notice that working with latest tf-nightly does not require this since updating the soft pin also invalidates the cache).