-
Notifications
You must be signed in to change notification settings - Fork 5
Continuation: use Lux.Training.single_train_step! #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* update CI * pretty * update k * be explicit * more f * formatting
* fix compat * rel path
|
I reworked the loss functions to match the signature Lux expects At least one of them. There is the other signature that @Qfl3x implemented but I think the one with these inputs
fits best to what we already had as |
|
Let's test more before we merge @lazarusA |
| @info "Check the saved output (.png, .mp4, .jld2) from training at: $(tmp_folder)" | ||
|
|
||
| prog = Progress(nepochs, desc = "Training loss", enabled = show_progress) | ||
| loss(hybridModel, ps, st, (x, y)) = lossfn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this is nice.
| struct MultiNNModel | ||
| struct MultiNNModel <: LuxCore.AbstractLuxContainerLayer{ | ||
| ( | ||
| :NNs, :predictors, :targets, :scale_nn_outputs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should dispatch on this, -> the NNs, instead of having 2 different structs, only one struct HybridModel end and an internal function HybridModel end that does the current constructHybridModel bit. For now, is good that this already works but we should address this in a following PR.
|
Yes, sounds good! Feel free to add or rm what you deem necessary. I also experimented a bit with making Enzyme work in the aforementioned script - but run into a problem/dead end with DimensionalData, so far: https://github.com/EarthyScience/EasyHybrid.jl/tree/ba/enzyme_tries |
yes, we need a new |
| struct MultiNNHybridModel | ||
| struct MultiNNHybridModel <: LuxCore.AbstractLuxContainerLayer{ | ||
| ( | ||
| :NNs, #:predictors, :forcing, :targets, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the interface should only deal with Layers, and not static inputs, see
https://lux.csail.mit.edu/stable/api/Building_Blocks/LuxCore#States
Zygote doesn't complain about it, but ForwardDiff will fail trying to apply preserves_state_type.
Leaving these as comments now, to be clean up later, once things are more stable they should be removed.
No description provided.