Fix openvino thresholds for export #21911
Conversation
Corrected indentation in doc string
Fixed issue with passing a single image without batch dimension.
…scale.py Co-authored-by: Jyotinder Singh <[email protected]>
Test case for unbatched inputs
Testcase for checking both unbatched and batched single image inputs.
There was a bug, and it was causing cycle in graph.
removed the use of tree.map_structure
…s-team#21254)" (keras-team#21329) This reverts commit 81821e0.
Enhanced the _can_use_flash_attention function to provide more detailed error messages when flash attention compatibility checks fail. Changes: - Replace generic exception catching with specific error propagation - When raise_error=True, directly re-raise original exceptions from check_layout() and check_is_flash_attention() functions - Preserve detailed error context from JAX internal validation functions - Maintain existing behavior when raise_error=False (returns False) This improves debugging experience by surfacing specific technical details about tensor layout incompatibilities, cuDNN version requirements, and other flash attention compatibility issues. Relates to keras-hub PR keras-team#2257 and addresses flash attention debugging needs.
… debugging" This reverts commit 7a0c547.
…sh_attention` Changes: - Add missing q_offsets=None and kv_offsets=None parameters to check_layout() call to match updated JAX function signature - Replace bare `except:` with `except Exception as e:` and `raise e` to preserve detailed error messages from JAX validation functions - Maintain existing fallback behavior when raise_error=False This resolves compatibility issues with newer JAX versions and improves debugging experience by surfacing specific technical details about flash attention compatibility failures.
Simplified the check for `flasth_attention` by removing redundant checks that are already done in `_can_use_flash_attention`.
- Updated assertAllClose calls in test_standard_model_export, test_model_with_input_structure, and test_model_with_multiple_inputs - Changed from default atol=1e-6, rtol=1e-6 to atol=1e-3, rtol=1e-3 - Fixes test failures due to numerical precision differences between Keras and OpenVINO inference
Summary of ChangesHello @pctablet505, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the stability and reliability of the OpenVINO export tests by introducing specific tolerance thresholds for numerical comparisons. By allowing for small, expected floating-point differences, the tests become more robust and less prone to spurious failures, ensuring that only significant deviations in model output are flagged. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the need for explicit tolerances in the OpenVINO export tests by adding atol and rtol to assertAllClose calls. This makes the tests more robust against minor floating-point discrepancies. I've added a few suggestions to define these tolerance values as constants to improve code maintainability by avoiding magic numbers. Overall, this is a good improvement.
| ov_output = compiled_model([ref_input])[compiled_model.output(0)] | ||
|
|
||
| self.assertAllClose(ref_output, ov_output) | ||
| self.assertAllClose(ref_output, ov_output, atol=1e-3, rtol=1e-3) |
There was a problem hiding this comment.
To improve maintainability and avoid magic numbers, consider defining the tolerance values 1e-3 as class-level constants. This makes it easier to understand their purpose and update them in one place if needed.
For example, you could add these attributes to the ExportOpenVINOTest class:
class ExportOpenVINOTest(testing.TestCase):
OV_ATOL = 1e-3
OV_RTOL = 1e-3
# ... tests ...Then, you can use them in your assertions like this:
self.assertAllClose(ref_output, ov_output, atol=self.OV_ATOL, rtol=self.OV_RTOL)
|
|
||
| ov_output = compiled_model(ov_inputs)[compiled_model.output(0)] | ||
| self.assertAllClose(ref_output, ov_output) | ||
| self.assertAllClose(ref_output, ov_output, atol=1e-3, rtol=1e-3) |
There was a problem hiding this comment.
Similar to my other comment, it would be better to use class-level constants for the tolerance values to improve maintainability and avoid repetition of magic numbers. For example: self.assertAllClose(ref_output, ov_output, atol=self.OV_ATOL, rtol=self.OV_RTOL).
| compiled_model.output(0) | ||
| ] | ||
| self.assertAllClose(ref_output, ov_output) | ||
| self.assertAllClose(ref_output, ov_output, atol=1e-3, rtol=1e-3) |
There was a problem hiding this comment.
Similar to my other comments, it would be better to use class-level constants for the tolerance values to improve maintainability and avoid repetition of magic numbers. For example: self.assertAllClose(ref_output, ov_output, atol=self.OV_ATOL, rtol=self.OV_RTOL).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21911 +/- ##
=======================================
Coverage 76.30% 76.30%
=======================================
Files 580 580
Lines 60031 60031
Branches 9433 9433
=======================================
Hits 45805 45805
Misses 11750 11750
Partials 2476 2476
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The tests are not failing at HEAD, so these are not needed. Maybe you're seeing this on your local machine only. |
4 participants
This pull request updates the OpenVINO export tests in
keras/src/export/openvino_test.pyto use explicit tolerances when comparing model outputs. This ensures that small numerical differences between reference and OpenVINO outputs are tolerated, making the tests more robust to minor floating-point discrepancies.Testing improvements:
self.assertAllClosecalls in the test cases to includeatol=1e-3andrtol=1e-3for output comparisons, allowing for small absolute and relative differences between reference and OpenVINO outputs. [1] [2] [3]