-
Notifications
You must be signed in to change notification settings - Fork 35
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
Adding padding at the input when necessary #342
base: main
Are you sure you want to change the base?
Conversation
Joao-L-S-Almeida
commented
Dec 23, 2024
- Should we add negative padding ? Could we remove some lines/rows instead of adding multiple lines/rows of zeroes ?
- Are we interested in choosing the kind of padding or 'constant' is good enough ?
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
tests/resources/configs/manufactured-finetune_prithvi_vit_100.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida on my side the tests are failing, can you please have a look? ==================================================================================== 30 failed, 530 passed, 6 skipped, 327 warnings in 1352.63s (0:22:32) ==================================================================================== |
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Are the errors looking like this ?
|
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@@ -1,8 +1,8 @@ | |||
# Copyright contributors to the Terratorch project | |||
|
|||
|
|||
from typing import List |
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.
It's better to use the types like list
instead of the typing
package. It's an old python package
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.
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@blumenstiel The argument output_size was removed. |
backbone: prithvi_eo_v2_300 | ||
# backbone_pretrained_cfg_overlay: | ||
# file: tests/prithvi_vit_300.pt | ||
backbone_drop_path_rate: 0.3 |
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.
Note that this parameter is called backbone_drop_path
for prithvi ViT (following the name of the parameter in the ViT block). backbone_drop_path_rate
is only working for prithvi swin.
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.
is prithvi_eo_v2_300 ViT or swin? @blumenstiel
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.
@romeokienzler Yes, all public pretrained models are ViT.
@@ -108,6 +109,7 @@ def build_model( | |||
|
|||
# Path for accessing the model source code. | |||
self.syspath_kwarg = "model_sys_path" | |||
backbone_kwargs, kwargs = extract_prefix_keys(kwargs, "backbone_") |
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.
@Joao-L-S-Almeida can you please update the docstring to explain the padding?
@@ -128,6 +130,17 @@ def build_model( | |||
backbone_kwargs, kwargs = extract_prefix_keys(kwargs, "backbone_") | |||
backbone = _get_backbone(backbone, **backbone_kwargs) | |||
|
|||
# If patch size is not provided in the config or by the model, it might lead to errors due to irregular images. |
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.
@Joao-L-S-Almeida can you please update the docstring to explain the padding?
/home/romeokienzler/gitco/terratorch/.venv/lib64/python3.12/site-packages/lightning/pytorch/cli.py:526: LightningCLI's args parameter is intended to run from within Python like if it were from the command line. To prevent mistakes it is not recommended to provide both args and command line arguments, got: sys.argv[1:]=['tests/'], args=['test', '-c', 'tests/resources/configs/manufactured-finetune_prithvi_pixelwise_pad.yaml']. tests/test_finetune.py::test_finetune_pad[validate-prithvi_eo_v2_300] tests/test_finetune.py::test_finetune_metrics_from_file[prithvi_swin_B] tests/test_generic_dataset.py::TestGenericSegmentationDataset::test_data_type_regression_float_long tests/test_prithvi_model_factory.py::test_create_pixelwise_model[UperNetDecoder-regression-expected0-prithvi_eo_v1_100] -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html |
@romeokienzler It seems to be torchgeo version trickery. It just works with the version from the requirements: |
Signed-off-by: Joao Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida sorry for the late reply, my build/test queue is fill with other branches, but will test before your morning... |