[compat] fix FusedDSAIndexerLoss compat mcore<0.19#142
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a version check for megatron.core to conditionally pass the calculate_per_token_loss parameter to FusedDSAIndexerLoss.apply only when using version 0.19.0rc0 or newer. The review feedback suggests using getattr when accessing calculate_per_token_loss from self.config to make the code more robust against potential AttributeErrors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| kwargs = {} | ||
| if mcore_019: | ||
| kwargs['calculate_per_token_loss'] = self.config.calculate_per_token_loss |
There was a problem hiding this comment.
Using getattr(self.config, 'calculate_per_token_loss', False) is safer and more robust than direct attribute access. This prevents potential AttributeError if self.config is a mock object in unit tests or a custom configuration class that does not define calculate_per_token_loss.
| kwargs['calculate_per_token_loss'] = self.config.calculate_per_token_loss | |
| kwargs['calculate_per_token_loss'] = getattr(self.config, 'calculate_per_token_loss', False) |
No description provided.