-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add Avalon.jl MLP Model #8
base: test
Are you sure you want to change the base?
Conversation
Closes #5 |
Notebook ready for review: Avalon.jl doesn't work in its current state so relevant primitives are copied over from repo. |
Script finished, ready for review |
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.
Not really familar with Julia but in general my main comments are around just structure and grouping things together to be a bit more clear what everything is doing.
For example the function Linear, Net struct and then the creation of the net object - I assume these are all for initializing the model structure, I think simply grouping them together, adding a bit more comments or even making a class for the model itself just makes it clear what these are doing, the others are pretty self explainitory.
The other example would be creating a train
function instead of just having it all in main().
Nothing major but these small structure changes I think would help with understading what each components is doing especially if you are unfamilar woth Julia.
I'll leave it up to our discrestion whether you want to make the changes here or just keep it in mind going forward.
Another side note, again I am not familar with the Julia ecosystem but what about enviroments and dependancies? At least there should be something sepcifying dependancies and the version used here. |
No description provided.