-
Notifications
You must be signed in to change notification settings - Fork 14
Simplify training routine #146
Comments
I like the way you're heading with this, I think it would also potentially translate down the road to a model agnostic repository that's a little more generalizable. Since it's come up a few times, I might add fixing the discontinuous sequences in #127 to the same update. |
I definitely think that #127 needs to be addressed, but I don't think it should be in this PR. I think our PRs should be able to be described in one line and have only one idea. This makes it easy to understand what we are trying to accomplish in any given PR at a glance. It also makes it clear when looking back in the git history what exactly a given PR was about. So I'm in favor of a 1) "simplify training routine" PR and 2) a separate "address non-continuous training sequences" PR, especially since they are independent components of the workflow. |
rm trainer class, model as input, files as input
this is to accommodate new train routine
also found bug in not passing spatial/time idx names all the way through
Also converted "trainer" class to more generic function
rm trainer class, model as input, files as input
this is to accommodate new train routine
also found bug in not passing spatial/time idx names all the way through
@SimonTopp made some improvements to the modularity of the training routine, and that got me thinking that it could be even simpler.
I'm thinking that we could just have basically the following parameters to the
train
function:Then all the other logic could go in the Snakefile including:
model
instead of str to training routine #118)Feel free to share thoughts here or it maybe clearer what I'm trying to do in the coming PR.
The text was updated successfully, but these errors were encountered: