search

gbm-developers / gbm3

Gradient boosted models
133 stars 117 forks source link

Add better handling of input NA #133

Open therneau opened 7 years ago

therneau commented 7 years ago

Add the nagbm.r function, which is essentially a clone of na.rpart, as the default na.omit method. It removes any obs that is missing y or is missing every predictor, i.e., the non-informative ones.

Modify the obs.id argument so that it first looks in the data= (this is where the subject identifier normally will be) and can accept any type of identifier. It is then turned into an integer before further processing.

pdmetcalfe commented 7 years ago

You're not going to like this, but could you add in some tests for this code?

therneau commented 7 years ago

You question implies that you actually see it -- and given the fight with git that makes it good news on the whole.

First a design question. Clearly observations with missing response should be tossed.
But is an obs that is missing all of the X variables completely uninformative in gbm?
Should we toss it?

Terry

On 12/09/2016 04:52 PM, Paul Metcalfe wrote:

You're not going to like this, but could you add in some tests for this code?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gbm-developers/gbm/pull/133#issuecomment-266145237, or mute the thread https://github.com/notifications/unsubscribe-auth/AJO3mRTZN9PFVgrQg_DjUlzgwR7boVR6ks5rGdu6gaJpZM4LJara.

pdmetcalfe commented 7 years ago

Looks like it's informative, so shouldn't be dropped.

therneau commented 7 years ago

Paul, I can fix that quickly in nagbm.r. Make it a new pull request, and you reject the prior one?

Terry T.

On 12/13/2016 01:20 PM, Paul Metcalfe wrote:

Looks like it's informative, so shouldn't be dropped.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gbm-developers/gbm/pull/133#issuecomment-266834572, or mute the thread https://github.com/notifications/unsubscribe-auth/AJO3mc7cUEY9JAMih7T53pTH4eKwsmgyks5rHu_5gaJpZM4LJara.

pdmetcalfe commented 7 years ago

You can push extra commits up to the same branch and it should update this pull request?

But do whatever's easiest for you...