Skip to content

Change LinearCyclicalScheduler to triangle wave to sawtooth wave#3186

Merged
vfdev-5 merged 14 commits intopytorch:masterfrom
sihyeong671:feat/scheduler
Feb 8, 2024
Merged

Change LinearCyclicalScheduler to triangle wave to sawtooth wave#3186
vfdev-5 merged 14 commits intopytorch:masterfrom
sihyeong671:feat/scheduler

Conversation

@sihyeong671
Copy link
Copy Markdown
Contributor

@sihyeong671 sihyeong671 commented Jan 11, 2024 •
edited by vfdev-5
Loading

Fixes #3185

Description:

Add monotonic variable in LinearCyclicalScheduler
LinearCyclicalScheduler's get_param method is modified

Add Test case LinearCyclicalScheduler ValueError
Add Test case LinearCyclicalScheduler with warmup duration

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: handlers Core Handlers module label Jan 11, 2024
@vfdev-5
Copy link
Copy Markdown
Collaborator

vfdev-5 commented Jan 11, 2024

Thanks for the PR and opening a corresponding feature request, @sihyeong671 !
The only problem I see here is BC-breaking change, so let's implement this feature as an optional flag with default value keeping current behavior, such that ignite users won't have their code broken after this PR get landed. What do you think?

@sihyeong671
Copy link
Copy Markdown
Contributor Author

sihyeong671 commented Jan 11, 2024 •
edited
Loading

Thanks for the PR and opening a corresponding feature request, @sihyeong671 ! The only problem I see here is BC-breaking change, so let's implement this feature as an optional flag with default value keeping current behavior, such that ignite users won't have their code broken after this PR get landed. What do you think?

got it! I think that is a good idea.I will try add optional flag

@sihyeong671
feat: change linearcyclicalscheduler
8d68161 
add optional flag for triangle and sawtooth wave
@sihyeong671
feat: fix LinearCyclicalScheduler
d091eea 
- move value error location to __init__
- rename use_legacy to use_sawtooth
@sihyeong671
test: add LinearCyclicalScheduler using use_sawtooth option test
6d10fde
@sihyeong671
Merge branch 'feat/scheduler' of https://github.com/sihyeong671/ignite …
75c3399 
…into feat/scheduler
@sihyeong671
Revert "test: add LinearCyclicalScheduler using use_sawtooth option t…
c3b88e0 
…est"

This reverts commit 6d10fde.
@sihyeong671
Merge branch 'master' of https://github.com/pytorch/ignite into feat/…
831db8d 
…scheduler
@sihyeong671
feat: Modify LinearCyclicalScheduler
86b7fed 
- add monotonic variable
- check Error case
@sihyeong671
test: add test case
22da23b 
- warmup_duration, monotonic Value Error Check
- add test case for linear cyclical scheduler with warmup_duration
@sihyeong671
fix: remove #noqa E501, split sentence
22d0475
vfdev-5
vfdev-5 reviewed Feb 5, 2024
View reviewed changes
vfdev-5
vfdev-5 reviewed Feb 5, 2024
View reviewed changes
vfdev-5
vfdev-5 reviewed Feb 5, 2024
View reviewed changes
vfdev-5
vfdev-5 approved these changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks a lot for the contribution, @sihyeong671 !

vfdev-5
vfdev-5 reviewed Feb 8, 2024
View reviewed changes
@vfdev-5 vfdev-5 merged commit 7fefe60 into pytorch:master Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vfdev-5 vfdev-5 vfdev-5 approved these changes

Assignees

No one assigned

Labels

module: handlers Core Handlers module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Changing LinearCyclicalScheduler triangle wave to sawtooth wave

2 participants

@sihyeong671 @vfdev-5