Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allowing to overwrite Prithvi and new segmentation tests. #68

Merged
merged 25 commits into from
Oct 14, 2024
Merged

Allowing to overwrite Prithvi and new segmentation tests. #68

merged 25 commits into from
Oct 14, 2024

Conversation

Joao-L-S-Almeida
Copy link
Member

@Joao-L-S-Almeida Joao-L-S-Almeida commented Aug 1, 2024 •
edited
Loading

Segmentation was not being tested during fine-tuning tests.

@Joao-L-S-Almeida
Testing backbones for segmentation
10f82fa 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Basic test for segmentation fine-tuning tasks
710f79e 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida Joao-L-S-Almeida self-assigned this Aug 1, 2024
@Joao-L-S-Almeida
Testing using more samples
023caf2 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Logging, remove it after the tests are finished
fdaeea3 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Merging updates from main
278c4d0 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Removing unnecessary logging
3fe4ac6 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
Joao-L-S-Almeida and others added 14 commits August 8, 2024 12:38
@Joao-L-S-Almeida
Logging
ee17c96 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
The number of channels can be estimated in different ways according t…
233a5c2 
…o the kind of bands definition

Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
terratorch.models.backbones.utils
6bd2bde 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
HLSBands also need be considered
52f0c12 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Better conditional for all the kinds of bands
3d9b5f5 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Allowing the configuration to overwrite default arguments from the mo…
5cf5876 
…del constructors

Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Intervals must include the upper extreme
ec8125e 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Merge pull request #106 from IBM/extend/bands
f4e9c47 
Extend/bands
@Joao-L-S-Almeida
Merge pull request #107 from IBM/overwrite_default_prithvi
cf963a8 
Overwrite default prithvi
@Joao-L-S-Almeida
Merge pull request #126 from IBM/extend/bands
7c6a5a2 
Extend/bands
@Joao-L-S-Almeida
Solving conflict in tests/test_finetune.py
28a8dfb 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Merge branch 'segmentation/tests' of github.com:IBM/terratorch into s…
5da9522 
…egmentation/tests
@Joao-L-S-Almeida
Run the tests again
8007bdf 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Merge branch 'main' into segmentation/tests
6055503
@Joao-L-S-Almeida
Copy link
Member Author

@CarlosGomes98 @romeokienzler The conflicts were solved.

@Joao-L-S-Almeida Joao-L-S-Almeida changed the title Segmentation/tests Allowing to overwrite Prithvi and new segmentation tests. Sep 5, 2024
CarlosGomes98
CarlosGomes98 suggested changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@CarlosGomes98 CarlosGomes98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the segmentation tests seems good to me. But the other two changes I see some issues with. Happy to discuss though

@Joao-L-S-Almeida
revert modification
0220ffc 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
This function is no more required
a7aac27 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
This function is no more required
b10d892 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
CarlosGomes98
CarlosGomes98 previously approved these changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@CarlosGomes98 CarlosGomes98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After removing that function (if it really is never used) lgtm. Lets do that ruff on the codebase after then :)

terratorch/models/backbones/utils.py Outdated
@@ -0,0 +1,15 @@
from terratorch.datasets import HLSBands

def _are_sublists_of_int(item) -> (bool, bool):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is never used. if so, can we remove it?

@CarlosGomes98 CarlosGomes98 dismissed their stale review September 9, 2024 10:26

function can be removed

@Joao-L-S-Almeida
merging
3232145 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
No more required.
7d4b4a5 
Signed-off-by: João Lucas de Sousa Almeida <[email protected]>
@Joao-L-S-Almeida
Copy link
Member Author

Done.

@Joao-L-S-Almeida
Copy link
Member Author

WARNING. If this PR isn't approved (or rejected) within 7 days it will be approved as is.

@Joao-L-S-Almeida Joao-L-S-Almeida merged commit ab6628f into main Oct 14, 2024
5 checks passed
@Joao-L-S-Almeida Joao-L-S-Almeida deleted the segmentation/tests branch October 14, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlosGomes98 CarlosGomes98 CarlosGomes98 left review comments

@romeokienzler romeokienzler Awaiting requested review from romeokienzler

Assignees

@Joao-L-S-Almeida Joao-L-S-Almeida

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Joao-L-S-Almeida @CarlosGomes98