Skip to content

Pass objective as passible engine argument for rule_fit()#96

Merged
EmilHvitfeldt merged 4 commits intomainfrom
pass-objective
Jan 26, 2026
Merged

Pass objective as passible engine argument for rule_fit()#96
EmilHvitfeldt merged 4 commits intomainfrom
pass-objective

Conversation

@topepo
Copy link
Member

@topepo topepo commented Jan 13, 2026

Related to #95

There are still issues that make it continue to be difficult to make the xrf and rules fits the same.

One other issue is that tidymodels uses a one-hot encoding when creating dummy variables for xgboost while xrf uses the standard full-rank encoding produced by model.matrix().

This PR solves the objective issue and updates some tests for the new xgboost API. The probability-based tests now use a tolerance since their results are very similar. I've removed or commented out tests based on hard class predictions, as we have no way of ensuring that those will be equal (but plan to resolve this discrepancy).

@topepo topepo requested a review from EmilHvitfeldt January 14, 2026 12:51
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt left a comment

Choose a reason for hiding this comment

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

very nice

Comment on lines -29 to -37
# use 4.0 or 4.1 to check with rtools40's older compiler
- {os: windows-latest, r: 'oldrel-4'}

- {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
- {os: ubuntu-latest, r: 'release'}
- {os: ubuntu-latest, r: 'oldrel-1'}
- {os: ubuntu-latest, r: 'oldrel-2'}
- {os: ubuntu-latest, r: 'oldrel-3'}
- {os: ubuntu-latest, r: 'oldrel-4'}
Copy link
Member

Choose a reason for hiding this comment

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

if these are related to the GHAs failing on older versions, I would use this trick instead: https://github.com/tidymodels/parsnip/pull/1317/files#diff-9c940e8ad2b7bc4c26ec3da57b94bc00e73e2166cfed689da51a4c59bcc0a310L59-L61

@@ -1,3 +1,20 @@
penalties <- 10^(-5:-1)
Copy link
Member

Choose a reason for hiding this comment

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

This appears not to be used anywhere

obs_pred <- rf_m_prob |> dplyr::filter(penalty == i)
for (i in 1:ncol(rf_prob)) {
expect_equal(obs_pred[[i]], unname(exp_pred[, i]))
expect_equal(obs_pred[[i]], unname(exp_pred[, i]), tolerance = 0.4)
Copy link
Member

Choose a reason for hiding this comment

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

that is quite the tolerance!

@EmilHvitfeldt EmilHvitfeldt merged commit 9799660 into main Jan 26, 2026
11 checks passed
@EmilHvitfeldt EmilHvitfeldt deleted the pass-objective branch January 26, 2026 23:14
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