Feature: add tests for meps dataset #38

SimonKamuk · 2024-05-22T13:57:32Z

Implemeted tests for loading a reduced size meps example dataset, creating graphs, and training model.

Work TODO:

reduce number of variables, size of domain etc in Joel's MEPS data example so that the zip file is less than 500MB. Calling it meps_example_reduced
create test-data zip file and upload to EWC (credentials from @leifdenby)
implement test using pytorch to download and unpack testdata using pooch
Implement testing of:
- initiation of neural_lam.weather_dataset.WeatherDataset from downloaded data
- check shapes of returned parts of training item
- create new graph in tests for reduced dataset
- feed single batch through model and check shape of output
add github action to run tests during ci/cd

closes #30

Also allowed calling train_model.main with arguments (will still use sys.argv when no arguments are supplied in the function call

Fixed error in plotting where non-callable cartopy projection from Config was called used current mesh generation from neural-lam instead of weather-model-graphs finished test of training call

…t will be installed anyway as a dependency during the other pip installs

…latex

leifdenby

Looks great to me! It will be really nice to have this automated testing on a real dataset. What do you think to creating a file called DEVELOPING.md in the repo root where you detail the contents of the test data a bit more?

@sadamov could you give this a read through too please?

tests/test_mllam_dataset.py

neural_lam/vis.py

neural_lam/utils.py

requirements.txt

tests/test_mllam_dataset.py

- removed linting dependencies - minor changes to test file - added notebook outlining generation of meps_example_reduced from meps_example

SimonKamuk · 2024-05-28T22:19:53Z

Looks great to me! It will be really nice to have this automated testing on a real dataset. What do you think to creating a file called DEVELOPING.md in the repo root where you detail the contents of the test data a bit more?

@sadamov could you give this a read through too please?

I added this as a notebook instead, since I could then include the code directly. Let me know if you would still prefer just a md file :)

sadamov · 2024-05-29T03:51:54Z

Looks great to me! It will be really nice to have this automated testing on a real dataset. What do you think to creating a file called DEVELOPING.md in the repo root where you detail the contents of the test data a bit more?
@sadamov could you give this a read through too please?

I added this as a notebook instead, since I could then include the code directly. Let me know if you would still prefer just a md file :)

Could we move this and all future notebooks to a neural-lam/notebooks folder? Then I would also give this notebook a more desciptive name like create_reduced_meps_dataset.ipynb

sadamov

Very nice @SimonKamuk, we definitely needed tests! Well done 🚀
I tested this with a push to mllam-repo and all tests run smoothly.

I think you should add a sentence or two about the new testing to https://github.com/mllam/neural-lam/blob/main/README.md#development-and-contributing
These tests take 150s (with download) and 100s(without) on my machine. I think that's okay since they mostly run in a seperate GitHub runner on push. In the future we could also use Mock objects for long-running tests (e.g. @mock.patch('train_model.main'))

Otherwise, I only requested some smaller changes. Thanks again Simon!

DEVELOPING.ipynb

tests/test_mllam_dataset.py

.github/workflows/run_tests.yml

leifdenby

I only have two minor comments about comments you could add to the code. Otherwise this is good to go.

Remember to update the changelog to say that you have added testing of dataset loading, graph creation and training based on reduced MEPS dataset stored on AWS S3! This is a big step!!

Also I think it would be nice to add to the top of the README cicd badges for the testing if @sadamov and @joeloskarsson you are ok with that?

DEVELOPING.ipynb

tests/test_mllam_dataset.py

joeloskarsson

This is very valuable to have! I had some questions about the added dependencies in requirements.py and also some follow-ups to earlier discussions that might be good to take a look at.

requirements.txt

neural_lam/utils.py

DEVELOPING.ipynb

joeloskarsson · 2024-05-29T16:55:38Z

I only have two minor comments about comments you could add to the code. Otherwise this is good to go.

Remember to update the changelog to say that you have added testing of dataset loading, graph creation and training based on reduced MEPS dataset stored on AWS S3! This is a big step!!

Also I think it would be nice to add to the top of the README cicd badges for the testing if @sadamov and @joeloskarsson you are ok with that?

I have no opinion on cicd badges, so please go ahead if you'd like

…add_tests

leifdenby

I think we just need to remove pytorch-geometric from the requirements.txt and then this is good to go!

…t action

joeloskarsson

All looking good now I think. I have only read the code, but trust that others have tested that everything runs. Great work on this!

…add_tests

leifdenby

Ready!

SimonKamuk · 2024-06-03T16:13:07Z

@sadamov Do you have any further requests or are we ready to merge? :)

sadamov · 2024-06-03T16:31:51Z

Ready go merge! 🚀

…add_tests

joeloskarsson · 2024-06-06T12:02:11Z

@SimonKamuk at the moment I am seeing "Unit Tests: Failing" on the badge, even thought this is not the case for the main branch. I was looking at https://github.com/mllam/neural-lam/actions/workflows/run_tests.yml and it seems to be on some other branch they have failed. I really don't know much about how these badges work, so not sure how it is supposed to function 😅 Do you know if there is a way to tweak it so that it only applies to the main branch? Or would that require large changes to the way the github workflows are set up?

SimonKamuk · 2024-06-06T14:10:43Z

@SimonKamuk at the moment I am seeing "Unit Tests: Failing" on the badge, even thought this is not the case for the main branch. I was looking at https://github.com/mllam/neural-lam/actions/workflows/run_tests.yml and it seems to be on some other branch they have failed. I really don't know much about how these badges work, so not sure how it is supposed to function 😅 Do you know if there is a way to tweak it so that it only applies to the main branch? Or would that require large changes to the way the github workflows are set up?

Yeah, I noticed that as well. I could specify that the badge should only refer to the main branch, but the issue is that the workflows are not running on pushes to main, so there are no runs to refer to. In the recent commit, I made these changes, but I am not sure what the implication of no longer ignoring pushes to main (why were they ignored in the first place @leifdenby?)

joeloskarsson · 2024-06-06T16:28:42Z

Ok, that's great! I guess running on push to main is generally unnecessary since we run everything before merging. However I don't think it matters if we run it an extra time just for the badge.

SimonKamuk and others added 3 commits May 22, 2024 15:36

added testing of loading data, creating graphs, and training model.

57edd74

Also allowed calling train_model.main with arguments (will still use sys.argv when no arguments are supplied in the function call

Merge branch 'mllam:main' into main

92aa490

added test to test name

4e17efb

SimonKamuk added this to the v0.2.0 milestone May 22, 2024

SimonKamuk added 13 commits May 22, 2024 16:01

linting

7fa7cdd

made create_mesh callable as python function with arguments.

569d061

Fixed error in plotting where non-callable cartopy projection from Config was called used current mesh generation from neural-lam instead of weather-model-graphs finished test of training call

added github ci/cd for running tests with pytest

1ebe900

removed coverage from test ci/cd

0e96e88

fixed error in cicd

2339ed0

removed astroid from requirements, causes codespell error, assuming i…

5d3f834

…t will be installed anyway as a dependency during the other pip installs

simplified requirements

8d733b7

removed commas in requirements

7ee8821

added downloading of test data from EWC using pooch

9a5f83c

added pooch to requirements.txt

c7d1d08

updated test dataset

2667b6c

Disabled latex to enable running on github without having to install …

0c7edd4

…latex

only use latex if available

9352949

SimonKamuk marked this pull request as ready for review May 27, 2024 12:18

SimonKamuk requested a review from leifdenby May 27, 2024 12:19

leifdenby requested changes May 28, 2024

View reviewed changes

leifdenby requested a review from sadamov May 28, 2024 10:16

leifdenby mentioned this pull request May 28, 2024

25 normalize data on gpu #39

Open

SimonKamuk added 2 commits May 29, 2024 00:00

included change requests from leifdenby:

4995de0

- removed linting dependencies - minor changes to test file - added notebook outlining generation of meps_example_reduced from meps_example

added comment

d33180f

sadamov requested changes May 29, 2024

View reviewed changes

DEVELOPING.ipynb Outdated Show resolved Hide resolved

tests/test_mllam_dataset.py Show resolved Hide resolved

tests/test_mllam_dataset.py Show resolved Hide resolved

.github/workflows/run_tests.yml Show resolved Hide resolved

leifdenby approved these changes May 29, 2024

View reviewed changes

DEVELOPING.ipynb Outdated Show resolved Hide resolved

tests/test_mllam_dataset.py Show resolved Hide resolved

joeloskarsson requested changes May 29, 2024

View reviewed changes

requirements.txt Outdated Show resolved Hide resolved

requirements.txt Outdated Show resolved Hide resolved

neural_lam/utils.py Show resolved Hide resolved

DEVELOPING.ipynb Outdated Show resolved Hide resolved

SimonKamuk and others added 4 commits May 30, 2024 18:55

Merge branch 'mllam:main' into main

fb72943

minor requested changes

e6c2c36

Merge branch 'main' of github.com:SimonKamuk/neural-lam into feature_…

43558dc

…add_tests

updated changelog, added cicd badges

3d77ac4

leifdenby requested changes May 31, 2024

View reviewed changes

SimonKamuk added 2 commits May 31, 2024 10:17

moved installation of torch-geometric from requirements to github tes…

d390308

…t action

changed name of unit test badge

de4efba

joeloskarsson approved these changes May 31, 2024

View reviewed changes

SimonKamuk added 5 commits June 3, 2024 07:11

added caching of test data

b0c4bed

Merge branch 'main' of github.com:SimonKamuk/neural-lam into feature_…

4868db4

…add_tests

fix for caching

18e55a4

tried fix for caching test data

4f75307

updated changelog

aceb47c

leifdenby approved these changes Jun 3, 2024

View reviewed changes

separated saving and restoring of cache

a6f8089

sadamov self-requested a review June 3, 2024 16:32

sadamov approved these changes Jun 3, 2024

View reviewed changes

Merge branch 'main' of github.com:SimonKamuk/neural-lam into feature_…

561a26e

…add_tests

SimonKamuk merged commit 81d0840 into mllam:main Jun 4, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: add tests for meps dataset #38

Feature: add tests for meps dataset #38

SimonKamuk commented May 22, 2024 •

edited by leifdenby

Loading

leifdenby left a comment

SimonKamuk commented May 28, 2024

sadamov commented May 29, 2024

sadamov left a comment

leifdenby left a comment •

edited

Loading

joeloskarsson left a comment

joeloskarsson commented May 29, 2024 •

edited

Loading

leifdenby left a comment

joeloskarsson left a comment

leifdenby left a comment

SimonKamuk commented Jun 3, 2024

sadamov commented Jun 3, 2024

joeloskarsson commented Jun 6, 2024

SimonKamuk commented Jun 6, 2024

joeloskarsson commented Jun 6, 2024

Feature: add tests for meps dataset #38

Feature: add tests for meps dataset #38

Conversation

SimonKamuk commented May 22, 2024 • edited by leifdenby Loading

leifdenby left a comment

Choose a reason for hiding this comment

SimonKamuk commented May 28, 2024

sadamov commented May 29, 2024

sadamov left a comment

Choose a reason for hiding this comment

leifdenby left a comment • edited Loading

Choose a reason for hiding this comment

joeloskarsson left a comment

Choose a reason for hiding this comment

joeloskarsson commented May 29, 2024 • edited Loading

leifdenby left a comment

Choose a reason for hiding this comment

joeloskarsson left a comment

Choose a reason for hiding this comment

leifdenby left a comment

Choose a reason for hiding this comment

SimonKamuk commented Jun 3, 2024

sadamov commented Jun 3, 2024

joeloskarsson commented Jun 6, 2024

SimonKamuk commented Jun 6, 2024

joeloskarsson commented Jun 6, 2024

SimonKamuk commented May 22, 2024 •

edited by leifdenby

Loading

leifdenby left a comment •

edited

Loading

joeloskarsson commented May 29, 2024 •

edited

Loading