Skip to content

fix: use local ev_values and wrap dict.values() in list()#8087

Open
hashwnath wants to merge 1 commit into
deepspeedai:masterfrom
hashwnath:fix/ev-values-attributeerror-7983
Open

fix: use local ev_values and wrap dict.values() in list()#8087
hashwnath wants to merge 1 commit into
deepspeedai:masterfrom
hashwnath:fix/ev-values-attributeerror-7983

Conversation

@hashwnath

Copy link
Copy Markdown

Summary

  • self.ev_values[i][0] on line 3106 references a nonexistent instance attribute, causing AttributeError at runtime when eigenvalue monitoring fires. The local variable ev_values (defined on line 3102) is the intended target.
  • dict.values() returns a dict_values view in Python 3 which is not subscriptable. Wrapping in list() makes it indexable.

Test plan

  • Verified self.ev_values has no definition or assignment anywhere in engine.py
  • Confirmed ev_values (local) is the correct variable from line 3102
  • Confirmed list() wrapping is needed for Python 3 dict_values subscriptability

Fixes #7983

Two bugs in the eigenvalue monitoring branch of `_take_model_step`:

1. `self.ev_values[i][0]` references a nonexistent instance attribute.
   The local variable `ev_values` (no `self.`) is the intended target.

2. `dict.values()` returns a dict_values view which is not subscriptable
   in Python 3. Wrapping in `list()` makes it indexable.

Fixes deepspeedai#7983

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hashwanth S <s.hashwanth531@gmail.com>
@hashwnath

Copy link
Copy Markdown
Author

Hey, I looked into this and the fix is straightforward. Two things going on here:

  1. Line 3106 uses self.ev_values[i][0] but there's no self.ev_values defined anywhere on the class. The local variable ev_values from line 3102 is what was intended.
  2. Even after fixing the variable name, dict.values() returns a view object in Python 3 that doesn't support indexing by position. Wrapping it in list() makes it subscriptable.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect variable name: self.ev_values

1 participant