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.

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

Issue 216: end-to-end CI #247

Merged
merged 33 commits into from
Dec 17, 2024
Merged

Conversation

SamuelBrand1
Copy link
Collaborator

@SamuelBrand1 SamuelBrand1 commented Dec 16, 2024

This PR closes #216.

Steps:

This PR adds a CI which installs relevant deps, then runs a test script which first creates fake test data from an exponentially mean-varying Poisson process and then runs the forecasting pipeline script. The target pathogen is covid-19 and the target state is "CA" (chosen arbitrarily).

@SamuelBrand1 SamuelBrand1 linked an issue Dec 16, 2024 that may be closed by this pull request
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 8.17%. Comparing base (9884204) to head (8314ba1).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##            main    #247      +/-   ##
========================================
+ Coverage   7.96%   8.17%   +0.20%     
========================================
  Files         20      21       +1     
  Lines       1343    1346       +3     
========================================
+ Hits         107     110       +3     
  Misses      1236    1236              
Flag Coverage Δ
hewr 22.53% <100.00%> (+0.51%) ⬆️
pipelines 0.00% <ø> (ø)
pyrenew_hew 1.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelBrand1
Copy link
Collaborator Author

Blocked by an error

raise ValueError("Data frame appears to have date gaps")

Which is unexpected because the data is generated in sequence.

I'll parse this tomorrow.

@SamuelBrand1
Copy link
Collaborator Author

That error was due to differing names of disease across different data sets. Onto creating eval data.

@SamuelBrand1
Copy link
Collaborator Author

SamuelBrand1 commented Dec 17, 2024

Through the eval data and into generate_epiweekly

Traceback (most recent call last):
File "/Users/samandfi/GitHub/CFA/pyrenew-hew/pipelines/forecast_state.py", line 499, in
main(**vars(args))
File "/Users/samandfi/GitHub/CFA/pyrenew-hew/pipelines/forecast_state.py", line 299, in main
generate_epiweekly(model_run_dir)
File "/Users/samandfi/GitHub/CFA/pyrenew-hew/pipelines/forecast_state.py", line 53, in generate_epiweekly
raise RuntimeError(f"generate_epiweekly: {result.stderr}")
RuntimeError: generate_epiweekly: b''

This was due to the pipeline wanting to be run from root dir

@SamuelBrand1
Copy link
Collaborator Author

pipeline now runs locally

These will trigger CI fails to avoid false CI passes
@SamuelBrand1
Copy link
Collaborator Author

This seems to be running but noting that without the __pycache__/ dir it takes a while to compile.

@SamuelBrand1
Copy link
Collaborator Author

I've tried more "minimalistic" settings e.g. fewer look back points, but occasionally you get fails due to bad sampling propagating something unexpected along the pipeline. At this moment I'm using 90 days data for inference and drawing a single 500/500 chain.

Exploring the fastest option that 1) doesn't fail stochastically, 2) still covers the pipeline sufficiently to avoid false CI passes would be a good issue.

@SamuelBrand1
Copy link
Collaborator Author

I think this is now erroring because epipredict and epiprocess are not loaded.

@SamuelBrand1 SamuelBrand1 marked this pull request as ready for review December 17, 2024 14:54
Copy link
Collaborator

@damonbayer damonbayer left a comment

Choose a reason for hiding this comment

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

@SamuelBrand1 After I added the quarto action, all checks are passing. On Teams you alluded to a potential other issue, so holding off on approval for now.

@SamuelBrand1
Copy link
Collaborator Author

@SamuelBrand1 After I added the quarto action, all checks are passing. On Teams you alluded to a potential other issue, so holding off on approval for now.

Nice! Thanks @damonbayer . The other issue was using deps that need to be installed with pak e.g. epipredict; I hadn't got to the Quarto problem yet.

This should be good to review now.

@SamuelBrand1 SamuelBrand1 mentioned this pull request Dec 17, 2024
@SamuelBrand1 SamuelBrand1 changed the title Issue 216: end-to-end CI (DRAFT) Issue 216: end-to-end CI Dec 17, 2024
Copy link
Collaborator

@damonbayer damonbayer left a comment

Choose a reason for hiding this comment

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

Thanks @SamuelBrand1. This is a huge improvement for the repo!

@damonbayer damonbayer linked an issue Dec 17, 2024 that may be closed by this pull request
@damonbayer damonbayer merged commit 0d79bd0 into main Dec 17, 2024
15 checks passed
@damonbayer damonbayer deleted the 149-generate-test-data-programmatically branch December 17, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants