Refitting an already fit ngboost estimator gives different results?

[kmedved]

In the course of doing some hyperparameter tuning, I have discovered that ngboost appears to give different model fits each time I refit an estimator if I don't reinstatiate it. Specifically, the first time I fit the model with early stopping enabled, the model fits in a reasonable number of iterations (depends on the data). Subsequent times however, early stopping kicks in after very few rounds however. This does not appear to be a random seed/non-deterministic issue, as when I reinstatiate the estimator, I get the same result each time.

This is surprising to me, as I was under the impression that when you refit an already fit estimator, you should expect a 'fresh' fit. That doesn't seem to be the case however with ngboost.

I have created a reproducible example here: https://colab.research.google.com/drive/1ZrVrPMkO5V0QZovnyyRt4bpKnBQPb8uv?usp=sharing (free to edit).

As a comparison, I checked how LightGBM handles this, and it does not display this behavior. The model terminates after the same number of rounds each time.

Is this to be expected? Is there something specific to the ngboost algorithm which is causing this? It's not a big deal per se, but I was pretty surprised to discover this, and want to make sure I'm not missing something.

[alejandroschuler]

Hmm, yeah I don't see a good reason for why the fit shouldn't be "fresh" each time. If you wanted to just add more estimators (trees) that should probably be a separate method (e.g. add_estimators(n_estimators)).

Fixing this should be pretty simple, it's just a matter of going through the code for NGBoost.fit(...) and explicitly resetting any instance variables it touches up at the top of the method to whatever they are set at by the NGBoost.__init__() before the rest of the code runs. Or that step could even be more cleanly put into another function e.g. NGBoost.reset() that gets called both by __init__() and at the top of fit() so everything is in one place. @kmedved do you want to try and see if you can do that and put in a PR?

[kmedved]

Happy to take a shot. I'm working on another PR elsewhere for early stopping as well, but haven't had time to finish that up, so it may take a bit.

To be clear, I don't think the current behavior is especially problematic; I only noticed it due to a peculiar hyperparameter tuning approach. I was mostly curious if this was intentional or not.

[alejandroschuler]

Thanks, and no hurry! Like you said, it's not a mission-critical bug, but it is somewhat unexpected behavior that, if unfixed, should at a minimum raise a warning for the user. It wasn't intentional on my part but perhaps @avati or @tonyduan had a different thought.