[GitHub] [tvm] comaniac commented on pull request #7992: Remove minimum seed constraint on XGB Tuner

[GitBox <gi...@apache.org>]

comaniac commented on pull request #7992:
URL: https://github.com/apache/tvm/pull/7992#issuecomment-833920681


   > @comaniac Maybe I'm misunderstanding the XBG tuner behavior -- I assumed that providing 10 records to seed the model is approximately functionally equivalent to the tuner exploring those 10 records on its own from scratch.
   > 
   > My use case is that if a tuning job is interrupted, I want to resume the progress midway. I assumed I could resume progress by invoking `load_history` with the records I'd seen so far. If seeding the records won't allow me to do this efficiently, do you know of another way?
   
   AFAIK by looking at the implementation, starting from the second iteration XGBTuner uses `self.xs` and `self.ys` to train the cost model. However, we didn't add loaded records to `self.xs` and `self.ys`, so they won't perform as you expected. If the goal is to support resume tuning, we need to add history records to `self.xs`,`self.ys`, and `self.visited` in `load_history`.
   
   > @comaniac even if we want a 500 default this should not be hardwired into the code, usually magic numbers are considered a bad coding practice, we should probably at least name this constant make this behavior configurable since 500 is arbitrary hyper parameter of the system
   
   I agree that 500 is ad-hoc and should be configurable, so I'm fine with exposing this number to the user API.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org