-
Notifications
You must be signed in to change notification settings - Fork 2
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
Sub-folders containing model-specific information #4
base: main
Are you sure you want to change the base?
Conversation
Inference, TATOR and Dockerfile still need to be tested.
Models can be downloaded from homepage
I think we should merge most of this to a new branch until after we sort out our future model zoo development strategy. We should definitely merge the new model into the table in the README of main. |
@Jordan-Pierce I added you as a contributor to our HuggingFace community. Do you want to try to PR the Spaces you set up into the FathomNet community there? It'd be cool to get a sense of what the process looks like for potential future model sharing in the HF. Thanks! |
Hey @eor314, I just transferred the spaces from my account to the FathomNet account. However, I just re-read your last comment about the PR specifically. How do you feel about me creating a Squid detector space under the FathomNet account, and while it's empty, I'll make a fork, add to the space, and then submit a PR? |
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.
Thanks so much for the PR Jordan! I left some minor comments, but overall I like the structure you've created.
A couple overarching thoughts:
- Including assets (images, videos) in the repo adds quite a bit of bloat, especially when they're duplicated across the different model subdirectories.
- Each model directory lends itself to being its own self-contained repo. I'd suggest breaking each model into a separate repo, then adding them to this one as submodules. That way, we can keep the organization of the models separate from the model assets/scripts themselves. As this repo grows, it may become unwieldy to manage all of them under one roof.
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 exclude IDE configuration from being checked in to the repo -- I recommend removing the .idea
directory and its contents
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.
Duplicated assets here.
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.
Ditto
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.
Ditto
Hey @eor314 @kevinsbarnard moving the conversation here; given the transition to HF do you just want to squash this PR? Or how would you like to proceed? I think the only addition I have to the readme is the inclusion of the UWROV in the table, which is also apart of the FathomNet HF, might be worth adding. |
Per the discussion, this PR contains the following:
Updated Main readme
Sub-folders for each model
Links to HuggingSpace demos are currently public, happy to transfer ownership and make a PR within the Fathomnet HuggingFace organization.
Take what you want, gut what you don't 😎