-
Notifications
You must be signed in to change notification settings - Fork 252
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 BASNet to keras hub #1984
Add BASNet to keras hub #1984
Conversation
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 for the PR Laxma! it looks great! can you add a demo colab to verify outputs? Thanks!
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.
Started a review. But I think this might need some bigger considerations.
Some questions:
- What are the weight sources we are using here?
- What are the outputs? Looks like there is a list of outputs instead of a normal classification style output.
- How does training this model look?
In general, I think there there will be a lot of core infrastructure that will break if the arch does not have a backbone class. DeepLabV3 is probably much closer to what we will need to go for.
But I think I need answers to the above questions to better suggest a design.
The weights conversion script uses the weights from https://keras.io/examples/vision/basnet_segmentation/ . There's also a Keras BASNet model available on Kaggle, but it failed to load for me so far, and I don't know if where it comes from, if it's better or if it's the same model anyway.
Yes, the direct model output is a list for refinement and prediction modules. The different stages of the model are trained with the same segmentation mask data. For inference, access to different stages of prediction modules would not be required for the user, but it might be interesting and doesn't hurt.
Training is simply performed on segmentation map data. For reference check this keras-cv notebook published in keras-io examples and issue |
Maybe for now just directly subclass resnet without changes as The broader question I have is how to make our |
I have created a |
Nice! Will take a look. Test failures look to be legit and basnet related, please take a look! |
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.
Let's try this out! I wonder if we will need to make more tweaks, having the num classes on the backbone itself is kinda funky, but do get why we are doing it for this architecture.
Left a number of style comments.
@laxmareddyp check the test failures! ~6 lines are too long and need fixing |
@mattdangerw I am modifying the code related to doc strings, other minor suggestions you mentioned and will push the changes and hope it will not show up this time. Thanks |
|
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.
Looks good overall. I'll drop the conversion script as I merge. Let's do that separately, need some work
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.
lgtm!
Refer this issue for more details
Please refer working demo notebook