Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Restructure of blocks + adding ConvLSTM #3

Open
wants to merge 3 commits into
base: dlesm
Choose a base branch
from
Open

Restructure of blocks + adding ConvLSTM #3

wants to merge 3 commits into from

Conversation

anna-dodson
Copy link
Collaborator

  • Divided blocks file into BasicConvBlock, ConvNeXt block, SymmetricConvNext, DoubleConvNext block
  • ConvGRUBlock, and ConvLSTM block now have their own files as well
  • pool, interpolate split apart too

Anna Dodson and others added 2 commits February 11, 2024 17:26
nathanielcresswellclay
nathanielcresswellclay reviewed Feb 13, 2024
View reviewed changes
Copy link
Owner

@nathanielcresswellclay nathanielcresswellclay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial look over the changes. I like the reorganization of our blocks but added a couple comments about some of the other changes. We can also talk about this when we meet tomorrow.

notebooks/Untitled-1.ipynb
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be in this PR? If it is please add a descriptive filename and some documentation about its purpose.

environments/zephyr-1.1.yml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the best practice is for this type of thing. Whoever uses the module needs to have write permissions to the "prefix" directory defined here. Can we use a relative path?

scripts/run_training.sh
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should leave the generic "run_training.sh" as a template script that trains our most recently published version of the model. Specific job scripts for different experiments probably don't need to be pushed to the repo. Open to you thoughts on this though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathanielcresswellclay nathanielcresswellclay nathanielcresswellclay left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anna-dodson @nathanielcresswellclay