Skip to content

Commit 99c331e

Browse files
dipannita08copybara-github
authored andcommitted
BEGIN_PUBLIC Enable GCM upload for slice efficiency metrics. END_PUBLIC
Tested: - monitoring_elastic_test (passed, + new unit test verifying GCM upload of slice efficiency metrics) - Test PyPi & E2E MaxText run (). PiperOrigin-RevId: 945844137
1 parent 393ec29 commit 99c331e

2 files changed

Lines changed: 175 additions & 4 deletions

File tree

ml_goodput_measurement/src/monitoring.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,8 @@ def _build_labels(labels_dict):
623623
'value_type': ValueType.DOUBLE,
624624
'metric_labels': _build_labels({
625625
'accelerator_type': gcp_options.acc_type,
626+
'window_type': 'CUMULATIVE',
627+
'rolling_window_size': '0',
626628
}),
627629
'resource_type': 'compute.googleapis.com/Workload',
628630
'resource_labels': {
@@ -632,7 +634,7 @@ def _build_labels(labels_dict):
632634
},
633635
})
634636
# TODO(b/519328677): Enable after metrics rollout
635-
# gcm_metrics.extend(slice_efficiency_metrics)
637+
gcm_metrics.extend(slice_efficiency_metrics)
636638

637639
# Send metrics to Google Cloud Monitoring.
638640
if metrics_sender and gcm_metrics:
@@ -813,6 +815,7 @@ def _build_labels(labels_dict):
813815
'metric_labels': _build_labels({
814816
'accelerator_type': gcp_options.acc_type,
815817
'rolling_window_size': str(window_size),
818+
'window_type': 'INTERVAL',
816819
}),
817820
'resource_type': 'compute.googleapis.com/Workload',
818821
'resource_labels': {
@@ -822,7 +825,7 @@ def _build_labels(labels_dict):
822825
},
823826
})
824827
# TODO(b/519328677): Enable after metrics rollout
825-
# gcm_metrics.extend(slice_efficiency_metrics)
828+
gcm_metrics.extend(slice_efficiency_metrics)
826829

827830
if metrics_sender and gcm_metrics:
828831
log_context = {

ml_goodput_measurement/tests/monitoring_elastic_test.py

Lines changed: 170 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,22 @@ def _create_timeseries(
5050
)
5151
return ts
5252

53+
def _compare_timeseries_ignore_time(self, expected_ts, actual_ts) -> bool:
54+
if expected_ts.metric.type != actual_ts.metric.type:
55+
return False
56+
if dict(expected_ts.metric.labels) != dict(actual_ts.metric.labels):
57+
return False
58+
if expected_ts.resource.type != actual_ts.resource.type:
59+
return False
60+
if dict(expected_ts.resource.labels) != dict(actual_ts.resource.labels):
61+
return False
62+
if len(expected_ts.points) != len(actual_ts.points):
63+
return False
64+
for p1, p2 in zip(expected_ts.points, actual_ts.points):
65+
if p1.value != p2.value:
66+
return False
67+
return True
68+
5369
def _compare_calls_ignore_time_series(
5470
self, expected_call, actual_call
5571
) -> bool:
@@ -62,8 +78,13 @@ def _compare_calls_ignore_time_series(
6278
for key, expected_value in expected_call.kwargs.items():
6379
actual_value = actual_call.kwargs[key]
6480
if key == 'time_series':
65-
continue
66-
if expected_value != actual_value:
81+
for exp_ts in expected_value:
82+
if not any(
83+
self._compare_timeseries_ignore_time(exp_ts, act_ts)
84+
for act_ts in actual_value
85+
):
86+
return False
87+
elif expected_value != actual_value:
6788
return False
6889

6990
return True
@@ -214,7 +235,154 @@ def test_upload_goodput_metrics_to_gcm_elastic_badput(
214235
f'Expected call not found: {expected_call}',
215236
)
216237

238+
@patch('google.cloud.monitoring_v3.MetricServiceClient')
239+
@patch('tensorboardX.writer.SummaryWriter')
240+
@patch('google.cloud.logging.Client')
241+
def test_upload_goodput_metrics_to_gcm_slice_efficiency(
242+
self,
243+
mock_logging_client,
244+
mock_summary_writer,
245+
mock_metric_service_client,
246+
):
247+
monitor = self._setup_mock_elastic_goodput_monitor(
248+
mock_logging_client, mock_summary_writer, mock_metric_service_client
249+
)
250+
mock_client = mock_metric_service_client.return_value
251+
252+
monitor._goodput_calculator.get_job_goodput_details = MagicMock(
253+
return_value={
254+
MetricType.GOODPUT_TIME.value: {
255+
GoodputType.TOTAL: 10.0,
256+
},
257+
MetricType.BADPUT_TIME.value: {
258+
BadputType.ELASTIC_SLICE_DOWN: 2.0,
259+
},
260+
MetricType.DISRUPTION_COUNT.value: 0,
261+
MetricType.MAX_PRODUCTIVE_STEP.value: 2,
262+
MetricType.TOTAL_ELAPSED_TIME.value: 20.0,
263+
MetricType.STEP_TIME_DEVIATION.value: {},
264+
MetricType.IDEAL_STEP_TIME.value: 1.0,
265+
'stepping_slice_efficiency': 0.8,
266+
'available_slice_efficiency': 0.9,
267+
}
268+
)
269+
270+
details = monitor._goodput_calculator.get_job_goodput_details()
271+
monitoring._upload_goodput_metrics_to_gcm(
272+
monitor._metrics_sender,
273+
details,
274+
monitor._worker_config,
275+
)
276+
277+
expected_calls = [
278+
mock.call.create_time_series(
279+
name='projects/test-project',
280+
time_series=[
281+
self._create_timeseries(
282+
'compute.googleapis.com/workload/stepping_slice_efficiency',
283+
{
284+
'accelerator_type': 'test-acc-type',
285+
'window_type': 'CUMULATIVE',
286+
'rolling_window_size': '0',
287+
},
288+
0.8,
289+
)
290+
],
291+
),
292+
mock.call.create_time_series(
293+
name='projects/test-project',
294+
time_series=[
295+
self._create_timeseries(
296+
'compute.googleapis.com/workload/available_slice_efficiency',
297+
{
298+
'accelerator_type': 'test-acc-type',
299+
'window_type': 'CUMULATIVE',
300+
'rolling_window_size': '0',
301+
},
302+
0.9,
303+
)
304+
],
305+
),
306+
]
307+
308+
actual_calls = mock_client.create_time_series.call_args_list
309+
310+
for expected_call in expected_calls:
311+
self.assertTrue(
312+
any(
313+
self._compare_calls_ignore_time_series(expected_call, actual)
314+
for actual in actual_calls
315+
),
316+
f'Expected call not found: {expected_call}',
317+
)
318+
319+
@patch('google.cloud.monitoring_v3.MetricServiceClient')
320+
@patch('tensorboardX.writer.SummaryWriter')
321+
@patch('google.cloud.logging.Client')
322+
def test_upload_interval_goodput_metrics_to_gcm_slice_efficiency(
323+
self,
324+
mock_logging_client,
325+
mock_summary_writer,
326+
mock_metric_service_client,
327+
):
328+
monitor = self._setup_mock_elastic_goodput_monitor(
329+
mock_logging_client, mock_summary_writer, mock_metric_service_client
330+
)
331+
mock_client = mock_metric_service_client.return_value
332+
333+
interval_metric_details = {
334+
IntervalMetricType.INTERVAL_SIZE.value: 3600,
335+
'stepping_slice_efficiency': 0.85,
336+
'available_slice_efficiency': 0.95,
337+
}
338+
339+
monitoring._upload_interval_goodput_metrics_to_gcm(
340+
monitor._metrics_sender,
341+
interval_metric_details,
342+
monitor._worker_config,
343+
)
344+
345+
expected_calls = [
346+
mock.call.create_time_series(
347+
name='projects/test-project',
348+
time_series=[
349+
self._create_timeseries(
350+
'compute.googleapis.com/workload/stepping_slice_efficiency',
351+
{
352+
'accelerator_type': 'test-acc-type',
353+
'rolling_window_size': '3600',
354+
'window_type': 'INTERVAL',
355+
},
356+
0.85,
357+
)
358+
],
359+
),
360+
mock.call.create_time_series(
361+
name='projects/test-project',
362+
time_series=[
363+
self._create_timeseries(
364+
'compute.googleapis.com/workload/available_slice_efficiency',
365+
{
366+
'accelerator_type': 'test-acc-type',
367+
'rolling_window_size': '3600',
368+
'window_type': 'INTERVAL',
369+
},
370+
0.95,
371+
)
372+
],
373+
),
374+
]
375+
376+
actual_calls = mock_client.create_time_series.call_args_list
217377

378+
for expected_call in expected_calls:
379+
self.assertTrue(
380+
any(
381+
self._compare_calls_ignore_time_series(expected_call, actual)
382+
for actual in actual_calls
383+
),
384+
f'Expected call not found: {expected_call}',
385+
)
218386

219387
if __name__ == '__main__':
220388
absltest.main()

0 commit comments

Comments
 (0)