Use builder pattern for model creation about burn HOT 9 CLOSED

Yes that's what I mean

from burn.

I'm closing this, since we have the init and init_with function on the config struct to build modules.

from burn.

The builder pattern is already implemented for configs, and I really want to put emphasis on using serializable hyper-parameters. However, we could still improve the API with an init function:

let conv = Conv2dConfig::new(1, 3) // Required arguments
                   .with_bias(false)
                   .init();

This would be a new Trait to be implemented by Module with something like:

// The derive can add `init()` to `Conv2dConfig` that dispatch to the `Initialize` trait.
#[derive(Module(config = "Conv2dConfig"))] 
pub struct Conv2d{
   ...
}

impl<B: Backend> Initialize<B> for Conv2d<B> {
    type Config = Conv2dConfig;
    
    fn init(config: &Self::Config) -> Self {
        ...
    }
    /// It would be cool to have an optional `init_state` function that would, by default, use the `init` method.
    fn init_state(state: &State<B::FloatElem>, config: &Self::Config) -> Self {
       ...
    }
}

We could require every module to implement Initialize and force them to have an associative type Config. With this, we could avoid having to initialize a module just before loading the weights (waste), which could reduce cold starts.

from burn.

Ho I forgot that you can't have arguments in derive macros. After consideration, I think we should just implement From.

impl<B: Backend> From<ModuleConfig> for Module<B> {
...
}

It automatically generate the into method, so you could have:

let module: Module<B> = ModuleConfig::new(...).into();

What I like about it is that it's very Rusty and you can have multiple From implementations for the same module. Which means that we could also implement:

impl<B: Backend> From<(ModuleConfig, State)> for Module<B> {
...
}

So instead of initializing a new module, it creates a new module with the given weights.

let module: Module<B> = (ModuleConfig:new(...), state).into();
// or
let module = Module::from((ModuleConfig::new(...), state));

It's a bit less pretty than the previous use case, since you have a tuple, but it's easy to understand I think.

Of course you don't always have to specify the type Module<B> if Rust can infer it, which may lighten up the syntax most of the time.

from burn.

@nathanielsimard

I feel like using from and into is a bit overloaded here. I think of into and from are used transform to a similar/sister structure but here we are trying to use a configuration to build a module.

Why is it necessary to do #[derive(Module(config = "Conv2dConfig"))] ?

Can we just do this?

impl<B: Backend> Initialize<B> for Conv2dConfig<B> {
    type Module = Conv2d;
    
    fn init(&self) -> Module {
        ...
    }
    fn init_state(&self, state: State) -> Module {
       ...
    }
}

It's similar as implementing From.

from burn.

The thing that bothers me with this is that some modules may need something else then the config to be initialized. For example, some modules may need shared parameters from another module before being initialized. So the Initialize trait would not fit that purpose. So do we need an abtraction for this knowing that not all modules will be able to implement it? I'm not sure.

Maybe we could just implement init and init_state on modules without being a trait. Also, the Initialize trait would not need to be imported to see the methods pop in autocomplete, maybe helping discoverability.

from burn.

Maybe we could just implement init and init_state on modules without being a trait. Also, the Initialize trait would not need to be imported to see the methods pop in autocomplete, maybe helping discoverability.

Do you mean init and init_state on *Config structs? Are we still able to do this?

let conv = Conv2dConfig::new(1, 3) // Required arguments
                   .with_bias(false)
                   .init();

with we can do this too:

let conv_config = Conv2dConfig::new(1, 3) // Required arguments
                   .with_bias(false);

let conv = conv_config.init(); 

// or 

let conv = conv_config.init_state(data); 

from burn.

@nathanielsimard Where do we stand on this task? Will you have additional check-ins on top what of your merged PR?

from burn.

I think we need to be able to load a model with the sate before the issue is 100% completed, probably very soon.

from burn.