Skip to content

Small fixes#99

Merged
vdplasthijs merged 14 commits intodevelopfrom
feature/set_up_encoders
Apr 30, 2026
Merged

Small fixes#99
vdplasthijs merged 14 commits intodevelopfrom
feature/set_up_encoders

Conversation

@gabrieletijunaityte
Copy link
Copy Markdown
Contributor

@gabrieletijunaityte gabrieletijunaityte commented Apr 24, 2026

What does this PR do?

  • Remove freezing predictive model twice (now it's done only from setup() method, found in BaseModel class.
  • Makes normalisation optional in PredictiveModel and EncoderWrapper to allow control and not normalise features twice.
    • Normalisation is parametrized.
    • Parameters are appended to the trainable modules list.
  • Fixed typo in geo_encoder trainable module list generation.
  • Added missing mode (train/val/test), appending it to the logging name (for loss and metrics). This is relevant to wandb tracking.
  • Encoder wrapper upon setup() call prints the model composition only once now.
  • Added aef data cehck (for missing, partial tiles).

Before submitting

  • Did you make sure title is self-explanatory and the description concisely explains the PR?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you test your PR locally with pytest command?

@gabrieletijunaityte gabrieletijunaityte requested review from robknapen and vdplasthijs and removed request for robknapen April 24, 2026 11:53
@gabrieletijunaityte gabrieletijunaityte marked this pull request as ready for review April 24, 2026 11:53
@gabrieletijunaityte gabrieletijunaityte marked this pull request as draft April 24, 2026 11:54
@gabrieletijunaityte gabrieletijunaityte changed the base branch from main to develop April 24, 2026 11:54
@gabrieletijunaityte gabrieletijunaityte marked this pull request as ready for review April 24, 2026 11:55
@gabrieletijunaityte
Copy link
Copy Markdown
Contributor Author

gabrieletijunaityte commented Apr 24, 2026

Sorry, at first I put the wrong target branch.

Copy link
Copy Markdown
Collaborator

@robknapen robknapen left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for finding and fixing these!

Copy link
Copy Markdown
Collaborator

@vdplasthijs vdplasthijs left a comment

Choose a reason for hiding this comment

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

Thanks! Can we change quiet to verbose with logic that if verbose>0 things should be printed, for consistency? Thanks!

Comment thread src/models/components/geo_encoders/base_geo_encoder.py Outdated
Comment thread src/models/components/geo_encoders/encoder_wrapper.py Outdated
Copy link
Copy Markdown
Collaborator

@vdplasthijs vdplasthijs left a comment

Choose a reason for hiding this comment

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

Awesome thanks!

@vdplasthijs vdplasthijs merged commit 9659b01 into develop Apr 30, 2026
4 checks passed
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.

3 participants