Skip to content

Commit 5edfab5

Browse files
committed
address comments
1 parent 6dceda0 commit 5edfab5

3 files changed

Lines changed: 16 additions & 21 deletions

File tree

src/maxtext/common/goodput.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
import contextlib
2323
import jax
2424
from enum import Enum
25+
import pathwaysutils
2526
from maxtext.utils import max_logging
26-
from maxtext.utils import pathwaysutils
2727
from maxtext.common.gcloud_stub import goodput_modules
2828

2929
goodput, monitoring, _GOODPUT_STUB = goodput_modules()
@@ -68,12 +68,13 @@ def maybe_monitor_goodput(config):
6868
enable_gcp_step_deviation_metrics=config.enable_gcp_step_deviation_metrics,
6969
)
7070
monitor_class = monitoring.GoodputMonitor
71-
use_elastic = getattr(config, 'elastic_enabled', False) and pathwaysutils.is_pathways_backend_used()
72-
if use_elastic:
71+
72+
if config.elastic_enabled:
7373
try:
7474
from ml_goodput_measurement import monitoring_elastic
7575
monitor_class = monitoring_elastic.ElasticGoodputMonitor
7676
except ImportError:
77+
max_logging.log("Elastic monitor failed!")
7778
pass
7879

7980
kwargs = dict(
@@ -138,9 +139,8 @@ def create_goodput_recorder(config):
138139
logger_name = f"goodput_{config.run_name}"
139140

140141
# Detect if we should use the elastic-aware recorder
141-
use_elastic = getattr(config, 'elastic_enabled', False) and pathwaysutils.is_pathways_backend_used()
142142
recorder = None
143-
if use_elastic:
143+
if config.elastic_enabled:
144144
try:
145145
from ml_goodput_measurement import goodput_elastic
146146
from maxtext.utils import elastic_utils

src/maxtext/utils/elastic_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def record_slice_state(recorder, active_slices_override: int | None = None) -> N
3535

3636
available_slices = len(pathwaysutils.elastic.get_active_slice_indices())
3737
active_slices = active_slices_override if active_slices_override is not None else len(elastic_manager.active_slice_indices)
38-
total_slices = len(elastic_manager.slice_to_devices)
38+
total_slices = len(pathwaysutils.elastic.get_slice_to_devices(jax.devices()))
3939

4040
recorder.record_elastic_slice_counts(
4141
available_slices=available_slices,

tests/unit/elastic_utils_test.py

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,11 @@ def test_record_elastic_event_start(self):
359359

360360
elastic_utils.record_elastic_event_start(fake_recorder, config)
361361

362-
fake_recorder.record_custom_badput_event_start_time.assert_called_once_with(
363-
custom_badput_event_type="elastic_slice_down"
362+
fake_recorder.record_elastic_wait_start_time.assert_called_once_with(
363+
event_type="elastic_slice_down"
364364
)
365365
fake_recorder.record_elastic_slice_counts.assert_called_once()
366366
self.assertEqual(elastic_utils.pending_elastic_event_type, 'elastic_slice_down')
367-
self.assertEqual(elastic_utils.pending_elastic_event_type, "elastic_slice_down")
368367

369368
def test_record_elastic_event_start_scale_up(self):
370369
"""Tests recording an elastic slice scale up start."""
@@ -376,8 +375,8 @@ def test_record_elastic_event_start_scale_up(self):
376375

377376
elastic_utils.record_elastic_event_start(fake_recorder, config)
378377

379-
fake_recorder.record_custom_badput_event_start_time.assert_called_once_with(
380-
custom_badput_event_type="elastic_scale_up"
378+
fake_recorder.record_elastic_wait_start_time.assert_called_once_with(
379+
event_type="elastic_scale_up"
381380
)
382381
fake_recorder.record_elastic_slice_counts.assert_called_once()
383382

@@ -388,8 +387,8 @@ def test_record_elastic_wait_end_and_reinit_start_noop_on_first_attempt(self):
388387

389388
elastic_utils.record_elastic_wait_end_and_reinit_start(fake_recorder)
390389

391-
fake_recorder.record_custom_badput_event_end_time.assert_not_called()
392-
fake_recorder.record_custom_badput_event_start_time.assert_not_called()
390+
fake_recorder.record_elastic_wait_end_time.assert_not_called()
391+
fake_recorder.record_elastic_reinit_start_time.assert_not_called()
393392
self.assertIsNone(elastic_utils.pending_reinit_recorder)
394393

395394
def test_record_elastic_wait_end_and_reinit_start(self):
@@ -403,12 +402,10 @@ def test_record_elastic_wait_end_and_reinit_start(self):
403402

404403
elastic_utils.record_elastic_wait_end_and_reinit_start(fake_recorder)
405404

406-
fake_recorder.record_custom_badput_event_end_time.assert_called_once_with(
407-
custom_badput_event_type="elastic_slice_down"
408-
)
409-
fake_recorder.record_custom_badput_event_start_time.assert_called_once_with(
410-
custom_badput_event_type="elastic_reinitialization"
405+
fake_recorder.record_elastic_wait_end_time.assert_called_once_with(
406+
event_type="elastic_slice_down"
411407
)
408+
fake_recorder.record_elastic_reinit_start_time.assert_called_once()
412409
fake_recorder.record_elastic_slice_counts.assert_called_once()
413410
self.assertIs(elastic_utils.pending_reinit_recorder, fake_recorder)
414411
self.assertIsNone(elastic_utils.pending_elastic_event_type)
@@ -423,9 +420,7 @@ def test_record_elastic_reinit_end(self):
423420

424421
elastic_utils.record_elastic_reinit_end()
425422

426-
fake_recorder.record_custom_badput_event_end_time.assert_called_once_with(
427-
custom_badput_event_type="elastic_reinitialization"
428-
)
423+
fake_recorder.record_elastic_reinit_end_time.assert_called_once()
429424
fake_recorder.record_elastic_slice_counts.assert_called_once()
430425
self.assertIsNone(elastic_utils.pending_reinit_recorder)
431426

0 commit comments

Comments
 (0)