Ignite: Remove deprecated CustomPeriodicEvent

[vfdev-5]

🚀 Feature

It is time to remove deprecated class CustomPeriodicEvent and corresponding related entries in the docs and the tests:

• remove file ignite/contrib/handlers/custom_events.py
• remove tests tests/ignite/contrib/handlers/test_custom_events.py
• other occurencies of CustomPeriodicEvent in other tests, code and docs

If you would like to help the project and work on this issue, please leave a message here. For any questions or guidance, feel free to ask here as well.

[Tawishi]

I would like to work on this issue.

[vfdev-5]

@Tawishi thanks ! This issue issue is very basic and consist to remove some deprecated code. Please, tell me if you are not familiar with github contributing flow (forking, local edits, PR etc) ? We have https://github.com/pytorch/ignite/blob/master/CONTRIBUTING.md which may help.

[Tawishi]

@vfdev-5 thanks for the prompt response. I am familiar with the basics. If I face a problem I will definitely ask.

[vfdev-5]

Sounds good! So, please, send a PR and we can iterate there for particular
details. Meanwhile, if you have any question, do not hesitate to ask here.

On Thu, Aug 27, 2020, 19:04 Tawishi [e-mail] wrote:

I am familiar with the basics.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/pytorch/ignite/issues/1247#issuecomment-682074886,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AASYOH3HWAVFUVQIWRTB5RDSC2G2BANCNFSM4QKLS6TQ
.

[Tawishi]

Sounds good! So, please, send a PR and we can iterate there for particular details. Meanwhile, if you have any question, do not hesitate to ask here.
…
On Thu, Aug 27, 2020, 19:04 Tawishi @.*> wrote: I am familiar with the basics. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1247 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASYOH3HWAVFUVQIWRTB5RDSC2G2BANCNFSM4QKLS6TQ .

@vfdev-5 I find it difficult to squash commits.

[vfdev-5]

@Tawishi thanks for the PR ! Actually, we do not require any squashing or linear commits history etc. While merging we squash all history in a single commit. Thus, no problems with that.

[vfdev-5]

@Tawishi just to give you a bit more context of what we are doing. CustomPeriodicEvent allows to do things like

from ignite.contrib.handlers import CustomPeriodicEvent

trainer = ...

# Let's define an event every 1000 iterations
cpe1 = CustomPeriodicEvent(n_iterations=1000)
cpe1.attach(trainer)

# Let's define an event every 10 epochs
cpe2 = CustomPeriodicEvent(n_epochs=10)
cpe2.attach(trainer)

@trainer.on(cpe1.Events.ITERATIONS_1000_COMPLETED)
def on_every_1000_iterations(engine):
     # run something every 1000 iterations
     print(engine.state.iterations_1000)

@trainer.on(cpe2.Events.EPOCHS_10_STARTED)
def on_every_10_epochs(engine):
    # run something every 10 epochs
    print(engine.state.epochs_10)

This nice but not very handy as we have to import class, create an instance and attach it to the trainer.
Then we introduced so-called filtered events with the API much simplier than this one

trainer = ...

@trainer.on(Events.ITERATION_COMPLETED(every=1000))
def on_every_1000_iterations(engine):
     # run something every 1000 iterations
     print(engine.state.iterations_1000)

@trainer.on(Events.EPOCH_STARTED(every=10))
def on_every_10_epochs(engine):
    # run something every 10 epochs
    print(engine.state.epochs_10)

So, we can achieve the same thing but almost 5 lines shorter. So, CustomPeriodicEvent was deprecated and now we can remove it. Hope this helps to understand better the context

[Tawishi]

@vfdev-5 What do I do in such situations where the result of CustomPeriodicEven is used in another function. Should I remove the entire function from the test ?

def test_pbar_on_custom_events(capsys):

    engine = Engine(update_fn)
    pbar = ProgressBar()
    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
        cpe = CustomPeriodicEvent(n_iterations=15)

    with pytest.raises(ValueError, match=r"not in allowed events for this engine"):
        pbar.attach(engine, event_name=cpe.Events.ITERATIONS_15_COMPLETED, closing_event_name=Events.EPOCH_COMPLETED)

[vfdev-5]

@Tawishi yes, you can safely remove those tests.

[Tawishi]

What about this exception handling code in tests/ignite/contrib/handlers/test_base_logger.py, @vfdev-5 ?

 with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
        n_iterations = 10
        cpe1 = CustomPeriodicEvent(n_iterations=n_iterations)
        n = len(data) * n_epochs / n_iterations
        nf = math.floor(n)
        ns = nf + 1 if nf < n else nf
        _test(cpe1.Events.ITERATIONS_10_STARTED, ns, cpe1)
        _test(cpe1.Events.ITERATIONS_10_COMPLETED, nf, cpe1)

    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
        n_iterations = 15
        cpe2 = CustomPeriodicEvent(n_iterations=n_iterations)
        n = len(data) * n_epochs / n_iterations
        nf = math.floor(n)
        ns = nf + 1 if nf < n else nf
        _test(cpe2.Events.ITERATIONS_15_STARTED, ns, cpe2)
        _test(cpe2.Events.ITERATIONS_15_COMPLETED, nf, cpe2)

    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
        n_custom_epochs = 2
        cpe3 = CustomPeriodicEvent(n_epochs=n_custom_epochs)
        n = n_epochs / n_custom_epochs
        nf = math.floor(n)
        ns = nf + 1 if nf < n else nf
        _test(cpe3.Events.EPOCHS_2_STARTED, ns, cpe3)
        _test(cpe3.Events.EPOCHS_2_COMPLETED, nf, cpe3)

[vfdev-5]

@Tawishi same here, just remove them as there is no more CustomPeriodicEvent. Thanks !

[Tawishi]

@Tawishi same here, just remove them as there is no more CustomPeriodicEvent. Thanks !

Thank you @vfdev-5 for guiding me through this ! :smile:

[vfdev-5]

@Tawishi seems like we forgot about this notebook example : https://github.com/pytorch/ignite/blob/master/examples/notebooks/EfficientNet_Cifar100_finetuning.ipynb - Cell 40.
Would you be interested in fixing that in another PR ? At the same it can be an opportunity to learn about Ignite's API and neural networks.

[Tawishi]

How should I start @vfdev-5 ?

[vfdev-5]

yes, please

On Sat, Aug 29, 2020, 09:43 Tawishi [e-mail] wrote:

How should I start @vfdev-5 https://github.com/vfdev-5 ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/pytorch/ignite/issues/1247#issuecomment-683252200,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AASYOH6QO2HDMYM7HR3HF23SDCWRLANCNFSM4QKLS6TQ
.