kristeligt-dagblad / dbt_ml

Package for dbt that allows users to train, audit and use BigQuery ML models.
Apache License 2.0
64 stars 26 forks source link

ML_CONFIG options won't work with numeric sequences #30

Closed mtronci closed 2 years ago

mtronci commented 2 years ago

Hi, I was trying to create a BQML model with the configuration below:

{{config( 
        materialized='model',
        ml_config={ 
                    'MODEL_TYPE': 'DNN_CLASSIFIER',
                    'INPUT_LABEL_COLS': ['will_default_1y'],
                    'HIDDEN_UNITS': [128, 16],
                    'ACTIVATION_FN': 'RELU',
                    'BATCH_SIZE': 32,
                    'DROPOUT': 0.1,
                    'EARLY_STOP': true,
                    'LEARN_RATE': 0.001,
                    'MAX_ITERATIONS': 50,
                    'OPTIMIZER': 'ADAGRAD'
                   } 
        ) 
    }}

The compilation step throws the following error:

15:22:37  Compilation Error in macro model_options (macros/materializations/model.sql)
15:22:37    'int object' has no attribute 'startswith'
15:22:37    
15:22:37    > in macro bigquery__create_model_as (macros/materializations/model.sql)
15:22:37    > called by macro create_model_as (macros/materializations/model.sql)
15:22:37    > called by macro statement (macros/etc/statement.sql)
15:22:37    > called by macro materialization_model_bigquery (macros/materializations/model.sql)
15:22:37    > called by macro model_options (macros/materializations/model.sql)

Amending the model.sql at line 33 from this:

                {%- if opt_val is sequence and (opt_val | first).startswith('hparam_') -%}

to this:

                {%- if opt_val is sequence and not (opt_val | first) is number and (opt_val | first).startswith('hparam_') -%}

seems to fix the issue.

Let me know if this makes sense and if it's helpful to submit a PR for this.

Thanks

rbjerrum commented 2 years ago

Thank you for opening the issue @mtronci. Your fix looks good - it would be very helpful if you submitted a PR for this.

rbjerrum commented 2 years ago

Closed by #31.