[Semantic Segmentation] Add first segmentation weights (credit @tanzhenyu) #1059
Conversation
|
Do you think this might be slightly improved with #968? In my previous experiences with different implementations, Dice does slightly worse than CE, but with our previous DLV3 run, it seemed like the loss was still high after training (0.9/1.0 as opposed to CE which had a significant decrease for a similar accuracy), so my intuition is that if the network is "aware" that it's still wrong, there might be untapped potential to boost the accuracy. |
| @@ -56,13 +64,19 @@ def __init__( | |||
| classes, | |||
| include_rescaling, | |||
| backbone, | |||
| weights, | |||
| backbone_weights=None, | |||
| weights=None, | |||
There was a problem hiding this comment.
do we want to provide default voc weights?
There was a problem hiding this comment.
I think we should offer defaults in the weights.py (as I have done with the aliases), but probably not in the parameter.
If you're referring to defaulting to None, I do think that's the right approach because it allows for random weight initialization by default.
There was a problem hiding this comment.
I mean default to voc, given keras.application models default to imagenet - https://www.tensorflow.org/api_docs/python/tf/keras/applications/resnet50/ResNet50
There was a problem hiding this comment.
Gotcha. Currently, our backbones don't have default weights. I don't personally love the idea of default weights, but I haven't thought about it a lot. Maybe we should discuss this as a package-wide option in our KerasCV sync?
Regardless of whether we include default weights, I would think we should keep them consistent across our model offerings.
There was a problem hiding this comment.
sounds good to me -- we could make this change later
| "weights file to be loaded, or a reference to pre-trained weights. " | ||
| f"Weights file not found at location: {weights}" | ||
| ) | ||
| self.load_weights(weights) |
There was a problem hiding this comment.
interesting...does this work? Today all our models are subclassed models, meaning the variables are not created at this point, I wonder if load_weights will error out
There was a problem hiding this comment.
Ahh yes I forgot that we have to call call on a subclass model before this will load.
So perhaps we want to actually override load_weights for these models to support our custom weights?
For now, I've removed the weights param from this PR, as we should probably discuss how we'll offer pre-trained non-backbone models.
IIRC KerasNLP has come up with a from_pretrained API that we might want to consider?
There was a problem hiding this comment.
the from_pretrained doesn't help much on this case, because KerasNLP actually creates all the variables during init. I have an action item to make our models to be function subclass models though, after which adding weights would be easier.
There was a problem hiding this comment.
sg -- for now would you like me to override load_weights?
There was a problem hiding this comment.
it's ok not to override, I will make this complete before 0.4 anyway
I think that it's likely there is still lots of opportunity to improve our semantic segmentation training script. Right now, I'm focused on getting the infrastructure set up to offer segmentation weights, using Zhenyu's VOC weights as a starting point. |
thanks! |
|
/gcbrun |
…enyu) (keras-team#1059) * Add first segmentation weights (credit @tanzheny) * Add training history * Remove leftover * Remove weights param * Comments
This also updates our training history updater to handle segmentation logs, and updates the deeplab implementation to load weights (also updates docstrings for deeplab)