Initialize XGBoost codegen #765
Conversation
sql/codegen_ant_xgboost.go
Outdated
| @@ -795,7 +795,7 @@ func xgCreatePredictionTable(pr *extendedSelect, r *antXGBoostFiller, db *DB) er | |||
| return nil | |||
| } | |||
|
|
|||
| func genXG(w io.Writer, pr *extendedSelect, ds *trainAndValDataset, fts fieldTypes, db *DB) error { | |||
| func genAntXGboost(w io.Writer, pr *extendedSelect, ds *trainAndValDataset, fts fieldTypes, db *DB) error { | |||
There was a problem hiding this comment.
how about renaming it to genAntXGBoost?
There was a problem hiding this comment.
You're right, that's a typo, thx.
| ValidationDatasetSQL string | ||
| TrainCfg *xgbTrainConfig | ||
| Features []*featureMeta | ||
| Label *featureMeta |
There was a problem hiding this comment.
I found our filler for tensorflow in codegen.go,
Lines 67 to 69 in 9a0dc86
Features named to X and Label to Y.I would suggest such consistent naming.
There was a problem hiding this comment.
Maybe a better way is reuse the filler in codegen, would optimize it in the next PR.
sql/expression_resolver_xgb.go
Outdated
| ParamsAttr map[string]*attribute | ||
| } | ||
|
|
||
| func resolveXGBTrainClause(tc *trainClause) (*resolvedXGBTrainClause, error) { |
There was a problem hiding this comment.
try put attribute extraction into the codegen for each submitter?
There was a problem hiding this comment.
It's used to resolve XGBoost parameters, the current expression_resover can only resolve the Tensorlfow parameters, maybe we can refactor the expression_resolver to extract getBoolAttr, getIntAttr as the common function.
There was a problem hiding this comment.
If we only have these two parameters currently, try all put it in expression_resolver.go and refine this later. We should let each codegen to deal with it's only attributes.
sql/expression_resolver_xgb.go
Outdated
| ParamsAttr map[string]*attribute | ||
| } | ||
|
|
||
| func resolveXGBTrainClause(tc *trainClause) (*resolvedXGBTrainClause, error) { |
There was a problem hiding this comment.
If we only have these two parameters currently, try all put it in expression_resolver.go and refine this later. We should let each codegen to deal with it's only attributes.
@typhoonzero , there is not only two parameters, and I removed the xgb resolver from this PR, would refactor the |
Initliaze XGBoost codegen, this PR reuse
db_generatorto generate XGBoost DMatrix object, part work of #749 .