diff --git a/ignite/contrib/engines/common.py b/ignite/contrib/engines/common.py index 31c241b2b18..847c8a95dd8 100644 --- a/ignite/contrib/engines/common.py +++ b/ignite/contrib/engines/common.py @@ -44,6 +44,7 @@ def setup_common_training_handlers( with_pbars: bool = True, with_pbar_on_iters: bool = True, log_every_iters: int = 100, + device: Optional[Union[str, torch.device]] = None, stop_on_nan: bool = True, clear_cuda_cache: bool = True, save_handler: Optional[Union[Callable, BaseSaveHandler]] = None, @@ -87,7 +88,10 @@ def setup_common_training_handlers( class to use to store ``to_save``. See :class:`~ignite.handlers.checkpoint.Checkpoint` for more details. Argument is mutually exclusive with ``output_path``. kwargs: optional keyword args to be passed to construct :class:`~ignite.handlers.checkpoint.Checkpoint`. + device: deprecated argument, it will be removed in v0.5.0. """ + if device is not None: + warnings.warn("Argument device is unused and deprecated. It will be removed in v0.5.0") if idist.get_world_size() > 1: _setup_common_distrib_training_handlers( diff --git a/ignite/engine/engine.py b/ignite/engine/engine.py index 5d5ab41f0b7..0912bbbc194 100644 --- a/ignite/engine/engine.py +++ b/ignite/engine/engine.py @@ -471,7 +471,7 @@ def state_dict_user_keys(self) -> List: return self._state_dict_user_keys def state_dict(self) -> OrderedDict: - """Returns a dictionary containing engine's state: "epoch_length", "max_epochs" and "iteration" and + """Returns a dictionary containing engine's state: "seed", "epoch_length", "max_epochs" and "iteration" and other state values defined by `engine.state_dict_user_keys` .. code-block:: python @@ -504,8 +504,8 @@ def save_engine(_): def load_state_dict(self, state_dict: Mapping) -> None: """Setups engine from `state_dict`. - State dictionary should contain keys: `iteration` or `epoch`, `max_epochs` and `epoch_length`. - If `engine.state_dict_user_keys` contains keys, they should be also present in the state dictionary. + State dictionary should contain keys: `iteration` or `epoch` and `max_epochs`, `epoch_length` and + `seed`. If `engine.state_dict_user_keys` contains keys, they should be also present in the state dictionary. Iteration and epoch values are 0-based: the first iteration or epoch is zero. This method does not remove any custom attributes added by user. @@ -604,12 +604,13 @@ def run( max_epochs: Optional[int] = None, max_iters: Optional[int] = None, epoch_length: Optional[int] = None, + seed: Optional[int] = None, ) -> State: """Runs the ``process_function`` over the passed data. Engine has a state and the following logic is applied in this function: - - At the first call, new state is defined by `max_epochs`, `max_iters`, `epoch_length`, if provided. + - At the first call, new state is defined by `max_epochs`, `max_iters`, `epoch_length`, `seed`, if provided. A timer for total and per-epoch time is initialized when Events.STARTED is handled. - If state is already defined such that there are iterations to run until `max_epochs` and no input arguments provided, state is kept and used in the function. @@ -629,6 +630,7 @@ def run( This argument should not change if run is resuming from a state. max_iters: Number of iterations to run for. `max_iters` and `max_epochs` are mutually exclusive; only one of the two arguments should be provided. + seed: Deprecated argument. Please, use `torch.manual_seed` or :meth:`~ignite.utils.manual_seed`. Returns: State: output state. @@ -657,6 +659,12 @@ def switch_batch(engine): trainer.run(train_loader, max_epochs=2) """ + if seed is not None: + warnings.warn( + "Argument seed is deprecated. It will be removed in 0.5.0. " + "Please, use torch.manual_seed or ignite.utils.manual_seed" + ) + if data is not None and not isinstance(data, Iterable): raise TypeError("Argument data should be iterable") diff --git a/ignite/engine/events.py b/ignite/engine/events.py index 16017fff009..c3c5affc4e0 100644 --- a/ignite/engine/events.py +++ b/ignite/engine/events.py @@ -1,4 +1,5 @@ import numbers +import warnings import weakref from enum import Enum from types import DynamicClassAttribute @@ -138,6 +139,17 @@ def __or__(self, other: Any) -> "EventsList": return EventsList() | self | other +class CallableEvents(CallableEventWithFilter): + # For backward compatibility + def __init__(self, *args: Any, **kwargs: Any) -> None: + super(CallableEvents, self).__init__(*args, **kwargs) + warnings.warn( + "Class ignite.engine.events.CallableEvents is deprecated. It will be removed in 0.5.0. " + "Please, use ignite.engine.EventEnum instead", + DeprecationWarning, + ) + + class EventEnum(CallableEventWithFilter, Enum): # type: ignore[misc] """Base class for all :class:`~ignite.engine.events.Events`. User defined custom events should also inherit this class. diff --git a/ignite/handlers/checkpoint.py b/ignite/handlers/checkpoint.py index 9a366c4f6ec..517685a672e 100644 --- a/ignite/handlers/checkpoint.py +++ b/ignite/handlers/checkpoint.py @@ -93,6 +93,7 @@ class Checkpoint(Serializable): Input of the function is ``(engine, event_name)``. Output of function should be an integer. Default is None, global_step based on attached engine. If provided, uses function output as global_step. To setup global step from another engine, please use :meth:`~ignite.handlers.global_step_from_engine`. + archived: Deprecated argument as models saved by ``torch.save`` are already compressed. filename_pattern: If ``filename_pattern`` is provided, this pattern will be used to render checkpoint filenames. If the pattern is not defined, the default pattern would be used. See Note for details. @@ -276,6 +277,7 @@ def __init__( score_name: Optional[str] = None, n_saved: Optional[int] = 1, global_step_transform: Optional[Callable] = None, + archived: bool = False, filename_pattern: Optional[str] = None, include_self: bool = False, greater_or_equal: bool = False, @@ -300,6 +302,8 @@ def __init__( if global_step_transform is not None and not callable(global_step_transform): raise TypeError(f"global_step_transform should be a function, got {type(global_step_transform)} instead.") + if archived: + warnings.warn("Argument archived is deprecated and will be removed in 0.5.0") self.to_save = to_save self.filename_prefix = filename_prefix @@ -755,6 +759,11 @@ class ModelCheckpoint(Checkpoint): Behaviour of this class has been changed since v0.3.0. + Argument ``save_as_state_dict`` is deprecated and should not be used. It is considered as True. + + Argument ``save_interval`` is deprecated and should not be used. Please, use events filtering instead, e.g. + :attr:`~ignite.engine.events.Events.ITERATION_STARTED(every=1000)` + There is no more internal counter that has been used to indicate the number of save actions. User could see its value `step_number` in the filename, e.g. `{filename_prefix}_{name}_{step_number}.pt`. Actually, `step_number` is replaced by current engine's epoch if `score_function` is specified and current iteration @@ -785,6 +794,7 @@ class ModelCheckpoint(Checkpoint): To setup global step from another engine, please use :meth:`~ignite.handlers.global_step_from_engine`. include_self: Whether to include the `state_dict` of this object in the checkpoint. If `True`, then there must not be another object in ``to_save`` with key ``checkpointer``. + archived: Deprecated argument as models saved by `torch.save` are already compressed. kwargs: Accepted keyword arguments for `torch.save` or `xm.save` in `DiskSaver`. .. versionchanged:: 0.4.2 @@ -812,17 +822,37 @@ def __init__( self, dirname: str, filename_prefix: str, + save_interval: Optional[Callable] = None, score_function: Optional[Callable] = None, score_name: Optional[str] = None, n_saved: Union[int, None] = 1, atomic: bool = True, require_empty: bool = True, create_dir: bool = True, + save_as_state_dict: bool = True, global_step_transform: Optional[Callable] = None, + archived: bool = False, include_self: bool = False, **kwargs: Any, ): + if not save_as_state_dict: + raise ValueError( + "Argument save_as_state_dict is deprecated and should be True." + "This argument will be removed in 0.5.0." + ) + if save_interval is not None: + msg = ( + "Argument save_interval is deprecated and should be None. This argument will be removed in 0.5.0." + "Please, use events filtering instead, e.g. Events.ITERATION_STARTED(every=1000)" + ) + if save_interval == 1: + # Do not break for old version who used `save_interval=1` + warnings.warn(msg) + else: + # No choice + raise ValueError(msg) + disk_saver = DiskSaver(dirname, atomic=atomic, create_dir=create_dir, require_empty=require_empty, **kwargs) super(ModelCheckpoint, self).__init__( @@ -833,6 +863,7 @@ def __init__( score_name=score_name, n_saved=n_saved, global_step_transform=global_step_transform, + archived=archived, include_self=include_self, ) diff --git a/tests/ignite/contrib/engines/test_common.py b/tests/ignite/contrib/engines/test_common.py index 5ad323a862f..8e5a493ceff 100644 --- a/tests/ignite/contrib/engines/test_common.py +++ b/tests/ignite/contrib/engines/test_common.py @@ -148,21 +148,8 @@ def test_asserts_setup_common_training_handlers(): train_sampler = MagicMock(spec=DistributedSampler) setup_common_training_handlers(trainer, train_sampler=train_sampler) - with pytest.raises(RuntimeError, match=r"This contrib module requires available GPU"): - setup_common_training_handlers(trainer, with_gpu_stats=True) - - with pytest.raises(TypeError, match=r"Unhandled type of update_function's output."): - trainer = Engine(lambda e, b: None) - setup_common_training_handlers( - trainer, - output_names=["loss"], - with_pbar_on_iters=False, - with_pbars=False, - with_gpu_stats=False, - stop_on_nan=False, - clear_cuda_cache=False, - ) - trainer.run([1]) + with pytest.warns(UserWarning, match=r"Argument device is unused and deprecated"): + setup_common_training_handlers(trainer, device="cpu") def test_no_warning_with_train_sampler(recwarn): diff --git a/tests/ignite/engine/test_custom_events.py b/tests/ignite/engine/test_custom_events.py index 6c9bf230dce..c16b34cfddf 100644 --- a/tests/ignite/engine/test_custom_events.py +++ b/tests/ignite/engine/test_custom_events.py @@ -6,7 +6,19 @@ import ignite.distributed as idist from ignite.engine import Engine, Events -from ignite.engine.events import CallableEventWithFilter, EventEnum, EventsList +from ignite.engine.events import CallableEvents, CallableEventWithFilter, EventEnum, EventsList + + +def test_deprecated_callable_events_class(): + engine = Engine(lambda engine, batch: 0) + + with pytest.warns(DeprecationWarning, match=r"Class ignite\.engine\.events\.CallableEvents is deprecated"): + + class CustomEvents(CallableEvents, Enum): + TEST_EVENT = "test_event" + + with pytest.raises(TypeError, match=r"Value at \d of event_names should be a str or EventEnum"): + engine.register_events(*CustomEvents) def test_custom_events(): diff --git a/tests/ignite/engine/test_engine.py b/tests/ignite/engine/test_engine.py index 672d6eadf23..2f9f3241fb6 100644 --- a/tests/ignite/engine/test_engine.py +++ b/tests/ignite/engine/test_engine.py @@ -362,6 +362,9 @@ def test_run_asserts(): with pytest.raises(ValueError, match=r"Input data has zero size. Please provide non-empty data"): engine.run([]) + with pytest.warns(UserWarning, match="Argument seed is deprecated"): + engine.run([0, 1, 2, 3, 4], seed=1234) + def test_state_get_event_attrib_value(): state = State() diff --git a/tests/ignite/handlers/test_checkpoint.py b/tests/ignite/handlers/test_checkpoint.py index af3f460ff54..b57cbc4afa5 100644 --- a/tests/ignite/handlers/test_checkpoint.py +++ b/tests/ignite/handlers/test_checkpoint.py @@ -58,6 +58,9 @@ def test_checkpoint_wrong_input(): with pytest.raises(TypeError, match=r"global_step_transform should be a function."): Checkpoint(to_save, lambda x: x, score_function=lambda e: 123, score_name="acc", global_step_transform=123) + with pytest.warns(UserWarning, match=r"Argument archived is deprecated"): + Checkpoint(to_save, lambda x: x, score_function=lambda e: 123, score_name="acc", archived=True) + with pytest.raises(ValueError, match=r"Cannot have key 'checkpointer' if `include_self` is True"): Checkpoint({"checkpointer": model}, lambda x: x, include_self=True) @@ -547,12 +550,24 @@ def test_model_checkpoint_args_validation(dirname): with pytest.raises(ValueError, match=r"with extension '.pt' are already present "): ModelCheckpoint(nonempty, _PREFIX) + with pytest.raises(ValueError, match=r"Argument save_interval is deprecated and should be None"): + ModelCheckpoint(existing, _PREFIX, save_interval=42) + with pytest.raises(ValueError, match=r"Directory path '\S+' is not found"): ModelCheckpoint(os.path.join(dirname, "non_existing_dir"), _PREFIX, create_dir=False) + with pytest.raises(ValueError, match=r"Argument save_as_state_dict is deprecated and should be True"): + ModelCheckpoint(existing, _PREFIX, create_dir=False, save_as_state_dict=False) + + with pytest.raises(ValueError, match=r"If `score_name` is provided, then `score_function` "): + ModelCheckpoint(existing, _PREFIX, create_dir=False, score_name="test") + with pytest.raises(TypeError, match=r"global_step_transform should be a function"): ModelCheckpoint(existing, _PREFIX, create_dir=False, global_step_transform=1234) + with pytest.warns(UserWarning, match=r"Argument archived is deprecated"): + ModelCheckpoint(existing, _PREFIX, create_dir=False, archived=True) + h = ModelCheckpoint(dirname, _PREFIX, create_dir=False) assert h.last_checkpoint is None with pytest.raises(RuntimeError, match=r"No objects to checkpoint found."):