Skip to content

Add support for Adam(W) and weight decay #1372

Merged
bw4sz merged 1 commit into
weecology:mainfrom
jveitchmichaelis:new-optimizer
Apr 16, 2026
Merged

Add support for Adam(W) and weight decay #1372
bw4sz merged 1 commit into
weecology:mainfrom
jveitchmichaelis:new-optimizer

Conversation

@jveitchmichaelis
Copy link
Copy Markdown
Collaborator

Description

Adds support for different optimizers in the config. Shouldn't break anything backwards as this was never exposed in older versions of DeepForest.

Fixes a small bug/footgun in the scheduler config where if you specified something not in the list, it would silently default to ReduceLROnPlateau.

Adam/w is widely used for training transformer-based models, and also supports a weight_decay parameter.

Related Issue(s)

In support of newer models being added to DeepForest.

AI-Assisted Development

  • I used AI tools (e.g., GitHub Copilot, ChatGPT, etc.) in developing this PR
  • I understand all the code I'm submitting
  • I have reviewed and validated all AI-generated code

AI tools used (if applicable):

Claude

@jveitchmichaelis jveitchmichaelis marked this pull request as ready for review April 14, 2026 23:46
@jveitchmichaelis jveitchmichaelis mentioned this pull request Apr 14, 2026
3 tasks
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.91%. Comparing base (408e150) to head (14d470c).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1372      +/-   ##
==========================================
+ Coverage   86.87%   86.91%   +0.03%     
==========================================
  Files          24       24              
  Lines        3064     3202     +138     
==========================================
+ Hits         2662     2783     +121     
- Misses        402      419      +17     
Flag Coverage Δ
unittests 86.91% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bw4sz
Copy link
Copy Markdown
Collaborator

bw4sz commented Apr 15, 2026

Yes, I've meant to do this. I sometimes wonder if there isn't a better kwargs type way of getting at pytorch-lightning, I don't love this pattern where we just wrap all things into the config. But I guess it does meet the style of trying to set reasonable defaults for users?

@bw4sz bw4sz self-requested a review April 15, 2026 22:06
Copy link
Copy Markdown
Collaborator

@bw4sz bw4sz left a comment

Choose a reason for hiding this comment

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

I'm approving this and we can discuss the larger pattern at Thursday meeting.

@jveitchmichaelis
Copy link
Copy Markdown
Collaborator Author

jveitchmichaelis commented Apr 15, 2026

Anything that's a trainer arg specifically can probably be a config group e.g. +trainer.strategy= ... and then we get CLI support for limit_batches and other things.

(Ie none of these options would be in the config, you'd pass a dict that gets added onto whatever is passed to the trainer, sort of like a **kwargs)

@bw4sz bw4sz merged commit da43450 into weecology:main Apr 16, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants