Bug fix for getting dataframes in TrainingJobAnalytics. (#441)

Author: piyushadlakha

CHANGELOG.rst

@@ -15,6 +15,7 @@ CHANGELOG
 * enhancement: Frameworks: update warning for not setting framework_version as we aren't planning a breaking change anymore
 * enhancement: Session: remove hardcoded 'training' from job status error message
 * bug-fix: Updated Cloudwatch namespace for metrics in TrainingJobsAnalytics
+* bug-fix: Changes to use correct s3 bucket and time range for dataframes in TrainingJobAnalytics.
 
 
 1.14.1

src/sagemaker/analytics.py

@@ -246,7 +246,12 @@ def _determine_timeinterval(self):
         """
         description = self._sage_client.describe_training_job(TrainingJobName=self.name)
         start_time = description[u'TrainingStartTime']  # datetime object
-        end_time = description.get(u'TrainingEndTime', datetime.datetime.utcnow())
+        # Incrementing end time by 1 min since CloudWatch drops seconds before finding the logs.
+        # This results in logs being searched in the time range in which the correct log line was not present.
+        # Example - Log time - 2018-10-22 08:25:55
+        #           Here calculated end time would also be 2018-10-22 08:25:55 (without 1 min addition)
+        #           CW will consider end time as 2018-10-22 08:25 and will not be able to search the correct log.
+        end_time = description.get(u'TrainingEndTime', datetime.datetime.utcnow()) + datetime.timedelta(minutes=1)
         return {
             'start_time': start_time,
             'end_time': end_time,

tests/unit/test_analytics.py

@@ -47,12 +47,49 @@ def create_sagemaker_session(describe_training_result=None, list_training_result
     cwm_mock = Mock(name='cloudwatch_client')
     boto_mock.client = Mock(return_value=cwm_mock)
     cwm_mock.get_metric_statistics = Mock(
-        name='get_metric_statistics',
-        return_value=metric_stats_results,
+        name='get_metric_statistics'
     )
+    cwm_mock.get_metric_statistics.side_effect = cw_request_side_effect
     return sms
 
 
+def cw_request_side_effect(Namespace, MetricName, Dimensions, StartTime, EndTime, Period, Statistics):
+    if _is_valid_request(Namespace, MetricName, Dimensions, StartTime, EndTime, Period, Statistics):
+        return _metric_stats_results()
+
+
+def _is_valid_request(Namespace, MetricName, Dimensions, StartTime, EndTime, Period, Statistics):
+    could_watch_request = {
+        'Namespace': Namespace,
+        'MetricName': MetricName,
+        'Dimensions': Dimensions,
+        'StartTime': StartTime,
+        'EndTime': EndTime,
+        'Period': Period,
+        'Statistics': Statistics,
+    }
+    print(could_watch_request)
+    return could_watch_request == cw_request()
+
+
+def cw_request():
+    describe_training_result = _describe_training_result()
+    return {
+        'Namespace': '/aws/sagemaker/TrainingJobs',
+        'MetricName': 'train:acc',
+        'Dimensions': [
+            {
+                'Name': 'TrainingJobName',
+                'Value': 'my-training-job'
+            }
+        ],
+        'StartTime': describe_training_result['TrainingStartTime'],
+        'EndTime': describe_training_result['TrainingEndTime'] + datetime.timedelta(minutes=1),
+        'Period': 60,
+        'Statistics': ['Average'],
+    }
+
+
 def test_abstract_base_class():
     # confirm that the abstract base class can't be instantiated directly
     with pytest.raises(TypeError) as _:  # noqa: F841
@@ -165,12 +202,15 @@ def test_trainer_name():
     assert str(trainer).find("my-training-job") != -1
 
 
-def test_trainer_dataframe():
-    describe_training_result = {
+def _describe_training_result():
+    return {
         'TrainingStartTime': datetime.datetime(2018, 5, 16, 1, 2, 3),
         'TrainingEndTime': datetime.datetime(2018, 5, 16, 5, 6, 7),
     }
-    metric_stats_results = {
+
+
+def _metric_stats_results():
+    return {
         'Datapoints': [
             {
                 'Average': 77.1,
@@ -186,8 +226,11 @@ def test_trainer_dataframe():
             },
         ]
     }
-    session = create_sagemaker_session(describe_training_result=describe_training_result,
-                                       metric_stats_results=metric_stats_results)
+
+
+def test_trainer_dataframe():
+    session = create_sagemaker_session(describe_training_result=_describe_training_result(),
+                                       metric_stats_results=_metric_stats_results())
     trainer = TrainingJobAnalytics("my-training-job", ["train:acc"], sagemaker_session=session)
 
     df = trainer.dataframe()