Skip to content

Standardise configs/models for comparison#241

Merged
jemrobinson merged 32 commits intomainfrom
model-comparison
Mar 25, 2026
Merged

Standardise configs/models for comparison#241
jemrobinson merged 32 commits intomainfrom
model-comparison

Conversation

@jemrobinson
Copy link
Copy Markdown
Member

  • Standardised parameters across:
    • cnn_null_cnn
    • cnn_unet_cnn
    • cnn_vit_cnn
    • ddpm
    • naive_null_naive
    • naive_unet_naive
    • naive_vit_naive
    • persistence
    • piecewise_null_piecewise
    • piecewise_unet_piecewise
    • piecewise_vit_piecewise
  • Added 'hemisphere' as a config option
    • This allows us to filter on it for train/evaluate jobs
  • Added type hints
  • Added configs for 14 day and 21 day predictions
  • Use base test_metrics, test_loss and val_loss in DDPM for easier comparison with other models

@jemrobinson jemrobinson requested a review from a team March 9, 2026 15:54
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 9, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  icenet_mp
  model_service.py
  icenet_mp/callbacks
  plotting_callback.py 97-104
  icenet_mp/data_loaders
  combined_dataset.py 91
  common_data_module.py 38-45, 90-97
  single_dataset.py
  icenet_mp/losses
  __init__.py
  weighted_bce_loss.py
  weighted_l1_loss.py
  weighted_mse_loss.py
  icenet_mp/models
  base_model.py
  ddpm.py 40, 164-167, 170-171, 183, 195-204, 301-306, 343-350, 390-407
  icenet_mp/models/encoders
  piecewise_encoder.py
  icenet_mp/models/processors
  unet.py
  vit.py
  icenet_mp/types
  typedefs.py
Project Total  

This report was generated by python-coverage-comment-action

@IFenton
Copy link
Copy Markdown
Contributor

IFenton commented Mar 20, 2026

Reviewing this

Copy link
Copy Markdown
Contributor

@IFenton IFenton left a comment

Choose a reason for hiding this comment

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

With your changes to the default latent space size etc. this now runs very slowly on my computer (0.11 it/s vs. 3.74 it/s). Would make sense to change some of those defaults for the sample runs?

I'm also curious as to why you changed how the hemisphere was being obtained?

@jemrobinson
Copy link
Copy Markdown
Member Author

  • hemisphere change is so that it shows up in W&B as a model parameter
  • these settings are the ones that give the "biggest" (in memory) model that can fit on an H100. Do you have a suggestion about what a smaller sample one would look like? Or should we add a single, new config file with minimal settings for local running?

@IFenton
Copy link
Copy Markdown
Contributor

IFenton commented Mar 23, 2026

these settings are the ones that give the "biggest" (in memory) model that can fit on an H100. Do you have a suggestion about what a smaller sample one would look like? Or should we add a single, new config file with minimal settings for local running?

The biggest shift seems to have been changing the size of the encoder latent space - when I changed that to the old values it was a much more reasonable speed. So maybe we just need to change that. As to how, maybe we have a sample.yaml config which overrides the settings, and change the base.yaml config to use the full dataset by default?

@jemrobinson
Copy link
Copy Markdown
Member Author

As to how, maybe we have a sample.yaml config which overrides the settings, and change the base.yaml config to use the full dataset by default?

Done in 54e36e1

Copy link
Copy Markdown
Contributor

@IFenton IFenton left a comment

Choose a reason for hiding this comment

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

LGTM. Would be good to add a quick note to the README about the new quick_test config

@jemrobinson
Copy link
Copy Markdown
Member Author

LGTM. Would be good to add a quick note to the README about the new quick_test config

Done in cde8d30

@jemrobinson jemrobinson merged commit 507c535 into main Mar 25, 2026
3 checks passed
@jemrobinson jemrobinson deleted the model-comparison branch March 25, 2026 12:32
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.

2 participants