Open
Conversation
afkanpour
reviewed
Apr 30, 2024
Collaborator
afkanpour
left a comment
afkanpour
left a comment
There was a problem hiding this comment.
Hi Sana. Thanks for putting this PR together. A few questions before I review it:
- Have you run experiments with these configs already? If so, did they generate reasonable results, i.e. relatively close to the papers' results?
- A lot of details in these configs are repetitive, which makes it easy to make mistakes and hard to review. Have you thought about factoring out common pieces into a config, and then build specific changes on top of that config? For example, the settings between [SSL]_paper.yaml and [SSL]_paper_synth.yaml is just the synthetic data and setting parts. Everything else should be the same. Also, configs between different SSL algorithms have a lot in common. So there could be a central config and other configs derived from it.
Reviewed all commit messages.
Reviewable status: 0 of 34 files reviewed, all discussions resolved
* add code * add extra linear scripts * added scripts * fix bugs * add inaturalist to dali * fix inaturalist dali * add places dali dataloader * add 150 scripts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
[Feature ]
Short Description
Add running scripts based on their papers.
This change is