Skip to content

Fix RematScope(mode=None) truthy bug in get_current_remat_mode - #23411

Open
Vansh-Sharmaa wants to merge 1 commit into
keras-team:masterfrom
Vansh-Sharmaa:fix-remat-scope
Open

Fix RematScope(mode=None) truthy bug in get_current_remat_mode#23411
Vansh-Sharmaa wants to merge 1 commit into
keras-team:masterfrom
Vansh-Sharmaa:fix-remat-scope

Conversation

@Vansh-Sharmaa

Copy link
Copy Markdown

Summary of Changes

Fixes an issue where RematScope(mode=None) was incorrectly evaluated as truthy when calling get_current_remat_mode().

Root Cause

RematScope accepts mode=None to explicitly disable rematerialization within a context block. Previously, get_current_remat_mode() returned a RematMode namedtuple (RematMode(mode=None, ...)), which evaluates to True in boolean context checks (if get_current_remat_mode():). Consequently, downstream code treated rematerialization as enabled even when mode=None was specified.

Solution

  • Updated get_current_remat_mode() in keras/src/backend/common/remat.py to return None when active_scope.mode is None.
  • Added a test assertion in TestRematScope.test_remat_scope_activation in keras/src/backend/common/remat_test.py verifying that get_current_remat_mode() returns None inside with RematScope(mode=None):.

Testing

  • Ran all tests in keras/src/backend/common/remat_test.py (6/6 passed).

@google-cla

google-cla Bot commented Aug 8, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the rematerialization utility to correctly handle cases where a RematScope is explicitly initialized with mode=None. Specifically, get_current_remat_mode() now returns None if the active scope's mode is None, and a corresponding unit test has been added to verify this behavior. I have no additional feedback to provide as the changes are clean, correct, and well-tested.

@codecov-commenter

codecov-commenter commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.15%. Comparing base (f585ff3) to head (365e912).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #23411      +/-   ##
==========================================
- Coverage   84.91%   84.15%   -0.76%     
==========================================
  Files         468      468              
  Lines       70693    70699       +6     
  Branches    11711    11711              
==========================================
- Hits        60027    59499     -528     
- Misses       7659     8204     +545     
+ Partials     3007     2996      -11     
Flag Coverage Δ
keras 83.98% <100.00%> (-0.74%) ⬇️
keras-cpu 83.98% <100.00%> (-0.01%) ⬇️
keras-gpu ?
keras-jax 58.07% <100.00%> (-0.34%) ⬇️
keras-numpy 53.88% <100.00%> (-0.01%) ⬇️
keras-openvino 59.63% <100.00%> (-0.01%) ⬇️
keras-tensorflow 59.74% <100.00%> (-0.30%) ⬇️
keras-torch 59.12% <100.00%> (-0.40%) ⬇️
keras-tpu ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants