From 99a3abe2bb0119ff994406363759232f5948ae43 Mon Sep 17 00:00:00 2001 From: maoyaomin Date: Tue, 29 Mar 2022 20:18:00 +0800 Subject: [PATCH] fix the logger info and exception message in profiler and summary --- .../python/mindspore/profiler/profiling.py | 11 ++-- .../mindspore/train/callback/_landscape.py | 60 ++++++++++--------- .../train/callback/_summary_collector.py | 53 ++++++++-------- .../train/summary/_summary_adapter.py | 18 ++++-- .../mindspore/train/summary/_writer_pool.py | 2 +- .../mindspore/train/summary/summary_record.py | 27 +++++---- .../train/summary/test_summary_collector.py | 11 ++-- 7 files changed, 99 insertions(+), 83 deletions(-) diff --git a/mindspore/python/mindspore/profiler/profiling.py b/mindspore/python/mindspore/profiler/profiling.py index b4fd062ad15..5dc640153bb 100644 --- a/mindspore/python/mindspore/profiler/profiling.py +++ b/mindspore/python/mindspore/profiler/profiling.py @@ -50,7 +50,7 @@ INIT_OP_NAME = 'Default/InitDataSetQueue' def _environment_check(): if c_expression.security.enable_security(): - raise RuntimeError("Profiler is not supported if compiled with \'-s on\'") + raise RuntimeError("Profiler is not supported when MindSpore is installed which compiled with \'-s on\'.") class Profiler: @@ -299,7 +299,8 @@ class Profiler: task_sink = os.getenv("GRAPH_OP_RUN") if task_sink and task_sink == "1": - logger.warning("Profiling is not supported when task is not sink.") + logger.warning(f"For '{self.__class__.__name__}', Profiling is not supported if set environment " + f"'GRAPH_OP_RUN' value to 1, which means model training task is not sink.") def _set_ascend_job_id(self, ascend_job_id): """Set output_path for offline parsing performance data.""" @@ -584,7 +585,8 @@ class Profiler: self._has_started = True self._has_started_twice = True else: - raise RuntimeError("MD Profiling has finished, repeated start and stop actions are not supported.") + raise RuntimeError("MindSpore Profiling has finished, repeated start and stop actions are not " + "supported.") else: raise RuntimeError("The profiler has already started. Use profiler.start() only when start_profile value " "is set to False.") @@ -648,7 +650,8 @@ class Profiler: if self._has_started: self._has_started = False else: - raise RuntimeError("The profiler has not started, so can not stop.") + raise RuntimeError("The profiler has not started, so can not stop. Please call the start() method " + "before calling the stop() method.") # No need to stop anything if parse profiling data offline if self._is_offline_parser(): diff --git a/mindspore/python/mindspore/train/callback/_landscape.py b/mindspore/python/mindspore/train/callback/_landscape.py index 937645ea533..38da96ca284 100644 --- a/mindspore/python/mindspore/train/callback/_landscape.py +++ b/mindspore/python/mindspore/train/callback/_landscape.py @@ -327,8 +327,9 @@ class SummaryLandscape: json_path = os.path.join(self._ckpt_dir, 'train_metadata.json') if not os.path.exists(json_path): - raise FileNotFoundError(f'train_metadata json file path not exists,' - f'please use summary_collector to collect information to create the json file') + raise FileNotFoundError(f'For "{self.__class__.__name__}", train_metadata.json file does not exist ' + f'under the path, please use summary_collector to collect information to ' + f'create the json file') with open(json_path, 'r') as file: data = json.load(file) self._check_json_file_data(data) @@ -766,68 +767,67 @@ class SummaryLandscape: metrics[key] = value.eval() return metrics - @staticmethod - def _check_unit(unit): + def _check_unit(self, unit): """Check unit type and value.""" check_value_type('unit', unit, str) if unit not in ["step", "epoch"]: - raise ValueError(f'Unit should be step or epoch, but got the: {unit}') + raise ValueError(f'For "{self.__class__.__name__}", the "unit" in train_metadata.json should be ' + f'step or epoch, but got the: {unit}') - @staticmethod - def _check_landscape_size(landscape_size): + def _check_landscape_size(self, landscape_size): """Check landscape size type and value.""" check_value_type('landscape_size', landscape_size, int) # landscape size should be between 3 and 256. if landscape_size < 3 or landscape_size > 256: - raise ValueError(f'Landscape size should be between 3 and 256, but got the: {landscape_size}') + raise ValueError(f'For "{self.__class__.__name__}", "landscape_size" in train_metadata.json should be ' + f'between 3 and 256, but got the: {landscape_size}') - @staticmethod - def _check_create_landscape(create_landscape): + def _check_create_landscape(self, create_landscape): """Check create landscape type and value.""" check_value_type('create_landscape', create_landscape, dict) for param, value in create_landscape.items(): if param not in ["train", "result"]: - raise ValueError(f'The key to create landscape should be in ["train", "result"], ' - f'but got the: {param}') + raise ValueError(f'For "{self.__class__.__name__}", the key of "create_landscape" should be in ' + f'["train", "result"], but got the: {param}.') if len(create_landscape) < 2: - raise ValueError(f'The key to create landscape should be train and result, ' - f'but only got the: {param}') + raise ValueError(f'For "{self.__class__.__name__}", the key of "create_landscape" should be train ' + f'and result, but only got the: {param}') check_value_type(param, value, bool) - @staticmethod - def _check_intervals(intervals): + def _check_intervals(self, intervals): """Check intervals type and value.""" check_value_type('intervals', intervals, list) for _, interval in enumerate(intervals): check_value_type('each interval in intervals', interval, list) #Each interval have at least three epochs. if len(interval) < 3: - raise ValueError(f'Each landscape interval should not be less than three, ' - f'but got the: {interval}.') + raise ValueError(f'For "{self.__class__.__name__}", the length of each list in "intervals" ' + f'should not be less than three, but got the: {interval}.') for j in interval: if not isinstance(j, int): - raise TypeError(f'Landscape interval value type should be int, ' - f'but got the: {type(j)}.') + raise TypeError(f'For "{self.__class__.__name__}", the type of each value in "intervals" ' + f'should be int, but got the: {type(j)}.') - @staticmethod - def _check_device_ids(device_ids): + def _check_device_ids(self, device_ids): """Check device_ids type and value.""" check_value_type('device_ids', device_ids, list) for i in device_ids: if not isinstance(i, int): - raise TypeError(f'Landscape device_ids type should be int, ' - f'but got the: {type(i)}.') + raise TypeError(f'For "{self.__class__.__name__}.gen_landscapes_with_multi_process", the parameter ' + f'"device_ids" type should be int, but got the: {type(i)}.') #device_id should be between 0 and 7. if i < 0 or i > 7: - raise ValueError(f'Landscape device_ids value should be between 0 and 7,but got {i}.') + raise ValueError(f'For "{self.__class__.__name__}.gen_landscapes_with_multi_process", the parameter ' + f'"device_ids" should be between 0 and 7,but got {i}.') def _check_collect_landscape_data(self, collect_landscape): """Check collect landscape data type and value.""" for param in collect_landscape.keys(): if param not in ["landscape_size", "unit", "num_samples", "create_landscape", "intervals"]: - raise ValueError(f'The key of collect landscape should be landscape_size, unit, num_samples' - f'create_landscape or intervals, but got the: {param}. ') + raise ValueError(f'For "{self.__class__.__name__}", the key of collect landscape should be ' + f'landscape_size, unit, num_samples create_landscape or intervals, ' + f'but got the: {param}. ') if "landscape_size" in collect_landscape: landscape_size = collect_landscape.get("landscape_size") self._check_landscape_size(landscape_size) @@ -862,11 +862,13 @@ class SummaryLandscape: for _, epochs in enumerate(epoch_group.values()): # Each epoch_group have at least three epochs. if len(epochs) < 3: - raise ValueError(f'This group epochs length should not be less than 3' + raise ValueError(f'For "{self.__class__.__name__}", the "epoch_group" in train_metadata.json, ' + f'length of each list in "epoch_group" should not be less than 3, ' f'but got: {len(epochs)}. ') for epoch in epochs: if str(epoch) not in model_params_file_map.keys(): - raise ValueError(f'The model_params_file_map does not exist {epoch}th checkpoint in intervals.') + raise ValueError(f'For "{self.__class__.__name__}", the "model_params_file_map" in ' + f'train_metadata.json does not exist {epoch}th checkpoint in intervals.') check_value_type('step_per_epoch', step_per_epoch, int) self._check_landscape_size(landscape_size) diff --git a/mindspore/python/mindspore/train/callback/_summary_collector.py b/mindspore/python/mindspore/train/callback/_summary_collector.py index 461f4fdde1e..972d7fbc36d 100644 --- a/mindspore/python/mindspore/train/callback/_summary_collector.py +++ b/mindspore/python/mindspore/train/callback/_summary_collector.py @@ -287,14 +287,14 @@ class SummaryCollector(Callback): def __exit__(self, *err): self._record.close() - @staticmethod - def _check_positive(name, value, allow_none=False): + def _check_positive(self, name, value, allow_none=False): """Check if the value to be int type and positive.""" if allow_none and value is None: return check_value_type(name, value, int) if value <= 0: - raise ValueError(f'For `{name}` the value should be greater than 0, but got `{value}`.') + raise ValueError(f'For "{self.__class__.__name__}", the value of `{name}` should be greater than 0, ' + f'but got `{value}`.') def _create_epoch_group(self, intervals): """Create epoch group.""" @@ -342,14 +342,15 @@ class SummaryCollector(Callback): logger.debug("Hyper config is not in system environment.") return auto_custom_lineage_data if len(hyper_config) > HYPER_CONFIG_LEN_LIMIT: - logger.warning("Hyper config is too long. The length limit is %s, the length of " - "hyper_config is %s." % (HYPER_CONFIG_LEN_LIMIT, len(hyper_config))) + logger.warning("The 'MINDINSIGHT_HYPER_CONFIG' of environment variable is too long. The length limit " + "is %s, the length of hyper_config is %s." % (HYPER_CONFIG_LEN_LIMIT, len(hyper_config))) return auto_custom_lineage_data try: hyper_config = json.loads(hyper_config) except (TypeError, JSONDecodeError) as exc: - logger.warning("Hyper config decode error. Detail: %s." % str(exc)) + logger.warning("The 'MINDINSIGHT_HYPER_CONFIG' of environment variable decode error. " + "Detail: %s." % str(exc)) return auto_custom_lineage_data custom_lineage_data = hyper_config.get("custom_lineage_data") @@ -365,52 +366,50 @@ class SummaryCollector(Callback): """Check action type.""" check_value_type('keep_default_action', action, bool) - @staticmethod - def _check_landscape_size(landscape_size): + def _check_landscape_size(self, landscape_size): """Check landscape size type and value.""" check_value_type('landscape_size', landscape_size, int) # landscape size should be between 3 and 256. if landscape_size < 3 or landscape_size > 256: - raise ValueError(f'Landscape size should be less than 256 and more than 3, ' - f'but got the: {landscape_size}') + raise ValueError(f'For "{self.__class__.__name__}", the "landscape_size" in collect_specified_data ' + f'should be less than 256 and more than 3, but got the: {landscape_size}') - @staticmethod - def _check_unit(unit): + def _check_unit(self, unit): """Check unit type and value.""" check_value_type('unit', unit, str) if unit not in ["step", "epoch"]: - raise ValueError(f'Unit should be step or epoch, but got the: {unit}') + raise ValueError(f'For "{self.__class__.__name__}", unit in collect_specified_data should be step ' + f'or epoch, but got the: {unit}.') - @staticmethod - def _check_create_landscape(create_landscape): + def _check_create_landscape(self, create_landscape): """Check create landscape type and value.""" check_value_type('create_landscape', create_landscape, dict) for param, value in create_landscape.items(): if param not in ["train", "result"]: - raise ValueError(f'The key to create landscape should be in ["train", "result"], ' - f'but got the: {param}') + raise ValueError(f'For "{self.__class__.__name__}", the key to create landscape should be in ' + f'["train", "result"], but got the: {param}.') check_value_type(param, value, bool) - @staticmethod - def _check_intervals(intervals): + def _check_intervals(self, intervals): """Check intervals type and value.""" check_value_type('intervals', intervals, list) for _, interval in enumerate(intervals): check_value_type('each interval inintervals', interval, list) if len(interval) < 3: - raise ValueError(f'Each landscape interval should not be less than three, ' - f'but got the: {interval}') + raise ValueError(f'For "{self.__class__.__name__}", each landscape interval should not be less ' + f'than three, but got the: {interval}') for j in interval: if not isinstance(j, int): - raise TypeError(f'Landscape interval value type should be int, ' + raise TypeError(f'For "{self.__class__.__name__}", landscape interval value type should be int, ' f'but got the: {type(j)}') def _check_collect_landscape_data(self, collect_landscape): """Check collect landscape data type and value.""" unexpected_params = set(collect_landscape) - set(self._DEFAULT_SPECIFIED_DATA.get("collect_landscape")) if unexpected_params: - raise ValueError(f'For `collect_landscape` the keys {unexpected_params} are unsupported, expect' - f'the follow keys: {list(self._DEFAULT_SPECIFIED_DATA["collect_landscape"].keys())}') + raise ValueError(f'For "{self.__class__.__name__}", the keys {unexpected_params} of `collect_landscape` ' + f'are unsupported, expect the follow keys: ' + f'{list(self._DEFAULT_SPECIFIED_DATA.get("collect_landscape").keys())}') landscape_size = collect_landscape.get("landscape_size", 40) self._check_landscape_size(landscape_size) unit = collect_landscape.get("unit", "step") @@ -436,7 +435,8 @@ class SummaryCollector(Callback): unexpected_params = set(specified_data) - set(self._DEFAULT_SPECIFIED_DATA) if unexpected_params: - raise ValueError(f'For `collect_specified_data` the keys {unexpected_params} are unsupported, ' + raise ValueError(f'For "{self.__class__.__name__}", the keys {unexpected_params} of ' + f'`collect_specified_data` are unsupported, ' f'expect the follow keys: {list(self._DEFAULT_SPECIFIED_DATA.keys())}') if 'histogram_regular' in specified_data: @@ -784,7 +784,8 @@ class SummaryCollector(Callback): # we assume that the first one is loss. loss = output[0] else: - logger.warning("The output type could not be identified, so no loss was recorded in SummaryCollector.") + logger.warning("The output type could not be identified, expect type is one of " + "[int, float, Tensor, list, tuple], so no loss was recorded in SummaryCollector.") self._is_parse_loss_success = False return None diff --git a/mindspore/python/mindspore/train/summary/_summary_adapter.py b/mindspore/python/mindspore/train/summary/_summary_adapter.py index 30d20df2d87..77a26aadf2e 100644 --- a/mindspore/python/mindspore/train/summary/_summary_adapter.py +++ b/mindspore/python/mindspore/train/summary/_summary_adapter.py @@ -179,13 +179,15 @@ def _nptype_to_prototype(np_value): } np_type = None if np_value is None: - logger.error("The numpy value is none") + logger.error("The numpy value in tensor of Summary is none") else: np_type = np_value.dtype.type proto = np2pt_tbl.get(np_type, None) if proto is None: - raise TypeError("No match for proto data type.") + raise TypeError("No match for proto data type, np_value type expect value is one of ['np.bool_', 'np.int8', " + "'np.int16', 'np.int32', 'np.int64', 'np.uint8', 'np.uint16', 'np.uint32', 'np.uint64', " + "'np.float16', 'np.float', 'np.float64'].") return proto @@ -207,11 +209,12 @@ def _fill_scalar_summary(tag: str, np_value, summary): summary.scalar_value = np_value.item() return True if np_value.size > 1: - logger.warning( + logger.info( f"The tensor is not a single scalar, tag = {tag}, ndim = {np_value.ndim}, shape = {np_value.shape}") summary.scalar_value = next(np_value.flat).item() return True - logger.error(f"There no values inside tensor, tag = {tag}, size = {np_value.size}") + logger.error(f"The size of Summary tensor should greater than 1, " + f"but got size = {np_value.size}, this means has no values inside tensor, ") return False @@ -354,12 +357,15 @@ def _fill_image_summary(tag: str, np_value, summary_image, input_format='NCHW'): """ logger.debug(f"Set({tag}) the image summary value") if np_value.ndim != 4 or np_value.shape[1] not in (1, 3): - logger.error(f"The value is not Image, tag = {tag}, ndim = {np_value.ndim}, shape={np_value.shape}") + logger.error(f"The dimension of Summary tensor should be 4 or second dimension should be 1 or 3, " + f"but got tag = {tag}, ndim = {np_value.ndim}, shape={np_value.shape}, " + f"which means Summary tensor is not Image.") return False if np_value.ndim != len(input_format): logger.error( - f"The tensor with dim({np_value.ndim}) can't convert the format({input_format}) because dim not same") + f"The tensor with dimension({np_value.ndim}) can't convert the format({input_format}) " + f"because dimension not same, the dimension should be {len(input_format)}.") return False if 0 in np_value.shape: diff --git a/mindspore/python/mindspore/train/summary/_writer_pool.py b/mindspore/python/mindspore/train/summary/_writer_pool.py index 36a36dd1a61..966ab9e2522 100644 --- a/mindspore/python/mindspore/train/summary/_writer_pool.py +++ b/mindspore/python/mindspore/train/summary/_writer_pool.py @@ -192,7 +192,7 @@ class WriterPool(ctx.Process): is_exit = True if not self._writers: - logger.warning("Can not find any writer to write summary data, " + logger.warning("Can not find any SummaryWriter to write summary data, " "so SummaryRecord will not record data.") is_exit = True diff --git a/mindspore/python/mindspore/train/summary/summary_record.py b/mindspore/python/mindspore/train/summary/summary_record.py index 25b690189a4..fbb09f24dfd 100644 --- a/mindspore/python/mindspore/train/summary/summary_record.py +++ b/mindspore/python/mindspore/train/summary/summary_record.py @@ -75,15 +75,15 @@ def process_export_options(export_options): unexpected_params = set(export_options) - set(_DEFAULT_EXPORT_OPTIONS) if unexpected_params: - raise ValueError(f'For `export_options` the keys {unexpected_params} are unsupported, ' + raise ValueError(f'For "SummaryRecord", the keys {unexpected_params} of "export_options" are unsupported, ' f'expect the follow keys: {list(_DEFAULT_EXPORT_OPTIONS.keys())}') for export_option, export_format in export_options.items(): unexpected_format = {export_format} - _DEFAULT_EXPORT_OPTIONS.get(export_option) if unexpected_format: raise ValueError( - f'For `export_options`, the export_format {unexpected_format} are unsupported for {export_option}, ' - f'expect the follow values: {list(_DEFAULT_EXPORT_OPTIONS.get(export_option))}') + f'For "SummaryRecord", the export_format {unexpected_format} of "export_options" are unsupported ' + f'for {export_option}, expect the follow values: {list(_DEFAULT_EXPORT_OPTIONS.get(export_option))}') for item in set(export_options): check_value_type(item, export_options.get(item), [str, type(None)]) @@ -178,7 +178,8 @@ class SummaryRecord: Validator.check_str_by_regular(file_suffix) if max_file_size is not None and max_file_size < 0: - logger.warning("The 'max_file_size' should be greater than 0.") + logger.warning(f"For '{self.__class__.__name__}', the 'max_file_size' should be greater than 0. " + f"but got value {max_file_size}.") max_file_size = None Validator.check_value_type(arg_name='raise_exception', arg_value=raise_exception, valid_types=bool) @@ -239,8 +240,8 @@ class SummaryRecord: """ mode_spec = 'train', 'eval' if mode not in mode_spec: - raise ValueError(f'For "{self.__class__.__name__}", {repr(mode)} is not a recognized mode, ' - f'expect mode is train or eval') + raise ValueError(f'For "{self.__class__.__name__}.set_mode", {repr(mode)} is not a ' + f'recognized mode, expect the parameter "mode" is "train" or "eval"') self._mode = mode def add_value(self, plugin, name, value): @@ -300,14 +301,16 @@ class SummaryRecord: """ if plugin in ('tensor', 'scalar', 'image', 'histogram'): if not name or not isinstance(name, str): - raise ValueError(f'For "{self.__class__.__name__}", {repr(name)} is not a valid tag name, ' - f'expect type is str.') + raise ValueError(f'For "{self.__class__.__name__}", the parameter "name" type should be str, ' + f'but got {type(name)}.') if not isinstance(value, Tensor): - raise TypeError(f'Expect the value to be Tensor, but got {type(value).__name__}') + raise TypeError(f'For "{self.__class__.__name__}", the parameter "value" expect to be Tensor, ' + f'but got {type(value).__name__}') np_value = _check_to_numpy(plugin, value) if name in {item['tag'] for item in self._data_pool[plugin]}: entry = repr(f'{name}/{plugin}') - logger.warning(f'{entry} has duplicate values. Only the newest one will be recorded.') + logger.warning(f'For "{self.__class__.__name__}.add_value", {entry} has duplicate values. ' + f'Only the newest one will be recorded.') data = dict(tag=name, value=np_value) export_plugin = '{}_format'.format(plugin) if self._export_options is not None and export_plugin in self._export_options: @@ -323,8 +326,8 @@ class SummaryRecord: elif plugin == PluginEnum.LANDSCAPE.value: self._data_pool[plugin].append(dict(tag=name, value=value.SerializeToString())) else: - raise ValueError(f'For "{self.__class__.__name__}", no such plugin of {repr(plugin)}, ' - f'expect value is one of [tensor, scalar, image, histogram, train_lineage, ' + raise ValueError(f'For "{self.__class__.__name__}.add_value", no such "plugin" of {repr(plugin)} ' + f', expect value is one of [tensor, scalar, image, histogram, train_lineage, ' f'eval_lineage, dataset_graph, custom_lineage_data, graph, landscape]') def record(self, step, train_network=None, plugin_filter=None): diff --git a/tests/ut/python/train/summary/test_summary_collector.py b/tests/ut/python/train/summary/test_summary_collector.py index ca372d014c5..ea62228790f 100644 --- a/tests/ut/python/train/summary/test_summary_collector.py +++ b/tests/ut/python/train/summary/test_summary_collector.py @@ -117,7 +117,8 @@ class TestSummaryCollector: if isinstance(collect_freq, int): with pytest.raises(ValueError) as exc: SummaryCollector(summary_dir=summary_dir, collect_freq=collect_freq) - expected_msg = f'For `collect_freq` the value should be greater than 0, but got `{collect_freq}`.' + expected_msg = f'For "SummaryCollector", the value of `collect_freq` should be greater than 0, ' \ + f'but got `{collect_freq}`.' assert expected_msg == str(exc.value) else: with pytest.raises(TypeError) as exc: @@ -163,7 +164,7 @@ class TestSummaryCollector: SummaryCollector(summary_dir, export_options=export_options) unexpected_format = {export_options.get("tensor_format")} - expected_msg = f'For `export_options`, the export_format {unexpected_format} are ' \ + expected_msg = f'For "SummaryRecord", the export_format {unexpected_format} of "export_options" are ' \ f'unsupported for tensor_format, expect the follow values: ' \ f'{list(_DEFAULT_EXPORT_OPTIONS.get("tensor_format"))}' @@ -244,7 +245,7 @@ class TestSummaryCollector: data = {'unexpected_key': True} with pytest.raises(ValueError) as exc: SummaryCollector(summary_dir, collect_specified_data=data) - expected_msg = f"For `collect_specified_data` the keys {set(data)} are unsupported" + expected_msg = f"the keys {set(data)} of `collect_specified_data` are unsupported" assert expected_msg in str(exc.value) @security_off_wrap @@ -254,7 +255,7 @@ class TestSummaryCollector: data = {'unexpected_key': "value"} with pytest.raises(ValueError) as exc: SummaryCollector(summary_dir, export_options=data) - expected_msg = f"For `export_options` the keys {set(data)} are unsupported" + expected_msg = f'the keys {set(data)} of "export_options" are unsupported' assert expected_msg in str(exc.value) @security_off_wrap @@ -505,7 +506,7 @@ class TestSummaryCollector: data = {'unexpected_key': "value"} with pytest.raises(ValueError) as exc: SummaryCollector(summary_dir, collect_specified_data={'collect_landscape': data}) - expected_msg = f"For `collect_landscape` the keys {set(data)} are unsupported" + expected_msg = f"the keys {set(data)} of `collect_landscape` are unsupported" assert expected_msg in str(exc.value) @security_off_wrap