Skip to content

ENH add periodic encoding in tabular learner#1264

Merged
jeromedockes merged 7 commits into
skrub-data:mainfrom
glemaitre:periodic_encoding_tabular_learner
Apr 1, 2025
Merged

ENH add periodic encoding in tabular learner#1264
jeromedockes merged 7 commits into
skrub-data:mainfrom
glemaitre:periodic_encoding_tabular_learner

Conversation

@glemaitre

@glemaitre glemaitre commented Mar 31, 2025

Copy link
Copy Markdown
Member

This PR adds periodic encoding by default when the estimator is a linear model

TODO

  • add tests
  • update the docstring of the tabular_learner
  • update the user guide
  • update the potential example impacted

@jeromedockes

Copy link
Copy Markdown
Member

should it be just for linear models? or would most non-tree-based models (and perhaps even tree-based ones) benefit from it?

@GaelVaroquaux

GaelVaroquaux commented Mar 31, 2025 via email

Copy link
Copy Markdown
Member

@glemaitre

Copy link
Copy Markdown
Member Author

I don't mind to have an else statement such that all "other models" will compute it.

@jeromedockes

Copy link
Copy Markdown
Member

That would be very interesting to investigate in benchmarks. In the meanwhile my hunch would be that spline or circular encoding is the safe bet when we don't know much about the learner: it might be a bit wasteful when the learner doesn't need it, but it shouldn't tank the quality of predictions, and it can really improve the predictions when the learner does need it. that's just a guess though I don't have experiments or a reference to back it up

@jeromedockes jeromedockes added this to the 0.5.2 milestone Apr 1, 2025

@jeromedockes jeromedockes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

great, thank you very much @glemaitre !

@jeromedockes jeromedockes merged commit b475c09 into skrub-data:main Apr 1, 2025
@jeromedockes jeromedockes changed the title ENH add periodic encoding by default with linear models ENH add periodic encoding in tabular learner Apr 1, 2025
@glemaitre

Copy link
Copy Markdown
Member Author

Lol thanks @jeromedockes ant @Vincent-Maladiere

rcap107 pushed a commit to rcap107/skrub that referenced this pull request Apr 2, 2025
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.

4 participants