holub008 / xrf

eXtreme RuleFit (sparse linear models on XGBoost ensembles)
Other
43 stars 13 forks source link

Corrected bug in evaluate_rule_dense_only #7

Closed yama1968 closed 5 years ago

yama1968 commented 5 years ago

Was a bug where you would generate expressions like: v4>=2 & v10<-3 which R would interpret as (v4>=2 & V10) <- 3 which generates an error. I added ' ' before and after '<' and '>=' to get v4 > 2 & v10 < -3

holub008 commented 5 years ago

Good find and fix looks good. Thanks!

yama1968 commented 5 years ago

Hi Karl,

Always a pleasure to contribute. The new version is working fine with this correction, and faster than previously in non sparse mode, but slower in sparse mode. Is that normal?

Thanks, Yannick

Le sam. 27 avr. 2019 à 17:24, Karl Holub notifications@github.com a écrit :

Good find and fix looks good. Thanks!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/holub008/xrf/pull/7#issuecomment-487294850, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY3PH55MBCFACLYFBJRYZ3PSRVZBANCNFSM4HI3ZNSQ .

holub008 commented 5 years ago

Interesting. That is not expected, as the sparse code path has not changed much. When I run the example from README (which is sparse) it takes 75 seconds, which is faster than earlier this year. But I'm sure it can vary with machine / dataset.

Do you have an example? I can switch between different versions to check for differences.

yama1968 commented 5 years ago

Hi Karl,

Please find an example, run against xrf 0.1.2. I have provided both the .Rmd, for which you need the credit card fraud dataset out of kaggle, and a generate pdf.

Enjoy! Yannick

Le sam. 27 avr. 2019 à 18:09, Karl Holub notifications@github.com a écrit :

Interesting. That is not expected, as the sparse code path has not changed much. When I run the example from README (which is sparse) it takes 75 seconds, which is faster than earlier this year. But I'm sure it can vary with machine / dataset.

Do you have an example? I can switch between different versions to check for differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/holub008/xrf/pull/7#issuecomment-487298384, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY3PHYZOUTGYGQYK6H5SILPSR3DPANCNFSM4HI3ZNSQ .

yama1968 commented 5 years ago

And the same run against my fork of the 0.9.0. Interestingly enough, the resulting average precision is the same, but it is running much faster with 0.1.2! What have you changed for that?

Regards, Yannick

Le mar. 30 avr. 2019 à 21:53, Yannick Martel ymartel@gmail.com a écrit :

Hi Karl,

Please find an example, run against xrf 0.1.2. I have provided both the .Rmd, for which you need the credit card fraud dataset out of kaggle, and a generate pdf.

Enjoy! Yannick

Le sam. 27 avr. 2019 à 18:09, Karl Holub notifications@github.com a écrit :

Interesting. That is not expected, as the sparse code path has not changed much. When I run the example from README (which is sparse) it takes 75 seconds, which is faster than earlier this year. But I'm sure it can vary with machine / dataset.

Do you have an example? I can switch between different versions to check for differences.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/holub008/xrf/pull/7#issuecomment-487298384, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY3PHYZOUTGYGQYK6H5SILPSR3DPANCNFSM4HI3ZNSQ .

holub008 commented 5 years ago

@yama1968 Could you email me the Rmd (karljholub@gmail.com)? It doesn't look like github carried the attachments through.

yama1968 commented 5 years ago

Sure! Done that. Yannick

Le mer. 1 mai 2019 à 00:32, Karl Holub notifications@github.com a écrit :

@yama1968 https://github.com/yama1968 Could you email me the Rmd ( karljholub@gmail.com)? It doesn't look like github carried the attachments through.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/holub008/xrf/pull/7#issuecomment-488139596, or mute the thread https://github.com/notifications/unsubscribe-auth/ABY3PHZIQMCRXD63LW7UFUDPTDCI3ANCNFSM4HI3ZNSQ .