Skip to content

Conversation

@acerioni
Copy link
Member

No description provided.

@acerioni acerioni requested a review from a team May 28, 2025 13:19
Copy link
Member

@GwenaelleSa GwenaelleSa left a comment

Choose a reason for hiding this comment

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

A few WIP comments left.
The new parameter in the config files should be described in the readme of the repo.

Copy link
Contributor

@cleherny cleherny left a comment

Choose a reason for hiding this comment

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

Do you want me to make the change here? Or do you?
Anyway, I'd like to modify the post-processing script for the quarry example to make it consistent with the proj-dqry. So I can make the change directly in this branch if it's easier.

Copy link
Member Author

@acerioni acerioni left a comment

Choose a reason for hiding this comment

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

I've suggested an updated. If accepted, edits to the configuration files can be discarded.

MODEL_ZOO_CHECKPOINT_URL = cfg['model_weights']['model_zoo_checkpoint_url']
else:
MODEL_ZOO_CHECKPOINT_URL = None
INIT_MODEL_WEIGHTS = cfg['model_weights']['init_model_weights']
Copy link
Member Author

Choose a reason for hiding this comment

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

I suggest to update as follows:

INIT_MODEL_WEIGHTS = cfg['model_weights'].get('init_model_weights', False)

which would set INIT_MODEL_WEIGHTS to False by default, should the parameter not be provided by the config file. This would allow us to avoid updating all the existing configuration files.

@stdl-s-sonarqube

This comment has been minimized.

Copy link
Member

@GwenaelleSa GwenaelleSa left a comment

Choose a reason for hiding this comment

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

It was getting messy, so I added to the mess by doing the modifications I asked for and approving the PR.
@cleherny Thank you for your proposition, but I think we can merge this PR much faster than the bigger one on the DQRY example, so it's worth to keep this separated.
@acerioni You comment brought a good idea. I implemented it the same way we did for other optional arguments of the OD, meaning with a if-else statement rather than the get method. The result should be the same if I understood you correctly.

I don't merge because it's not very catholic knowing I was the last to modify this branch, but if @acerioni or @cleherny you agree with the modification, feel free to do it.

@stdl-s-sonarqube
Copy link

Passed

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 1 Code Smell

Coverage and Duplications

  • Coverage No coverage information (0.00% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: swiss-territorial-data-lab_object-detector_AYZ4zWIzr7JdaaSXwexc

View in SonarQube

@acerioni
Copy link
Member Author

Thanks @GwenaelleSa for your latest edits 👍

@acerioni acerioni merged commit c4e77b5 into master Sep 23, 2025
2 checks passed
@GwenaelleSa GwenaelleSa deleted the init_weights branch September 24, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants