Differential Binarization model

[mehtamansi29]

Differential Binarization model.

Notebook: https://colab.sandbox.google.com/gist/mehtamansi29/86289c0ef273772f5de309ce20d0292d/diffbin_training_2.ipynb

[sachinprasadhs]

Took high level pass and left some comments.
Also,
Make al the file names in follow the same format like other files, for db_utils and losses.py

[gemini-code-assist]

Code Review

This pull request introduces the Differential Binarization (DiffBin) model, including the model's backbone, text detector task, loss function, preprocessor, and associated utilities and tests. Additionally, there's a widespread cleanup of import statements across the API, removing redundant aliases, which improves code readability.

The implementation of the new DiffBin model is a great start, but there are several areas that need attention to improve correctness, maintainability, and completeness.

[gemini-code-assist]

The implementation of the loss function is confusing and seems to deviate from the original "Differentiable Binarization" paper, which could lead to incorrect behavior or difficulty in maintenance.

Specifically:

• Variable Naming: The variable names lb and lt seem to be swapped compared to their purpose. lb is calculated on threshold maps, while lt is calculated on binary maps. The paper uses L_b for binary map loss and L_t for threshold map loss.
• Hyperparameter Roles: The roles of alpha and beta are also swapped compared to the paper's formula L = L_s + alpha * L_b + beta * L_t. In the code, alpha weights the threshold map loss (lb) and beta weights the binary map loss (lt).
• Loss Functions: The paper suggests L1 loss for the threshold map and Dice loss for the binary map. This implementation uses BCE for the threshold map and L1 for the binary map.

While deviations from the paper can be valid, the current implementation is hard to follow due to the inconsistencies in naming. I recommend refactoring for clarity.

[gemini-code-assist]

The example in the docstring demonstrates a call to detector.postprocess_to_polygons(map_output[...,0]). However, the postprocess_to_polygons method is not implemented in the DiffBinTextDetector class. This method should be implemented, or the docstring should be updated to reflect the available functionality.

[gemini-code-assist]

Using mutable default arguments like lists or tuples is a common pitfall in Python and can lead to unexpected behavior. It's a best practice to default to None and then assign the mutable object inside the function body.

Consider adding the following logic at the beginning of __init__:

if head_kernel_list is None:
    head_kernel_list = [3, 2, 2]
if image_shape is None:
    image_shape = (640, 640, 3)

Suggested change

	self,
	image_encoder,
	fpn_channels=256,
	head_kernel_list=[3, 2, 2],
	image_shape=(640, 640, 3),
	dtype=None,
	head_kernel_list=None,
	image_shape=None,

[sachinprasadhs]

you can keep image_shape=(None, None, 3) for consistency

[gemini-code-assist]

This block of commented-out code should be removed to improve code clarity and maintainability.

[gemini-code-assist]

This upsampling layer with a size of (1, 1) is a no-op and can be removed to improve code clarity and avoid an unnecessary operation.

[gemini-code-assist]

The assertion self.assertGreaterEqual(loss.numpy(), 0.0) is very weak for this test case. When the predicted values are identical to the true values, the loss should be zero (or very close to it). A more precise assertion would make this test more meaningful and robust.

Suggested change

	loss = self.loss_fn(y_true, y_pred)
	self.assertGreaterEqual(loss.numpy(), 0.0)
	self.assertAllClose(loss.numpy(), 0.0, atol=1e-6)

[gemini-code-assist]

There is a redundant call to self.image_converter(x). The variable x has already been updated with the result of self.image_converter(x) on line 54. This line can be simplified to return x.

            return x

[sachinprasadhs]

Rebase the code to the latest master and resolve the gemini suggested comments