Skip to content

CI: Cache the earth_day_01d_p grid - #2937

Merged
seisman merged 3 commits into
mainfrom
cache/earth_day
Jan 2, 2024
Merged

CI: Cache the earth_day_01d_p grid#2937
seisman merged 3 commits into
mainfrom
cache/earth_day

Conversation

@seisman

Copy link
Copy Markdown
Member

Description of proposed changes

The earth_day_01d_p.tif file is used in test_grdimage_image.py.

@seisman

Copy link
Copy Markdown
MemberAuthor

One quick thought when working on this PR.

Currently, the "Cache Data" workflow is disabled by default for PRs. Sometimes, when we add a new cache file, we need to manually enable the workflow in PRs to refresh the caches and also need to disable it before merging. It's tedious although it doesn't happen very often.

A better solution is:

  1. Move the download_test_data function to a separate source file, e.g., pygmt/helpers/caching.py.
  2. Enable the workflow if pygmt/helpers/caching.py changes:
pull_request:
paths:
- 'pygmt/helpers/caching.py'

@seismanseisman added this to the 0.11.0 milestone Jan 1, 2024
@seismanseisman added maintenance Boring but important stuff for the core devs needs review This PR has higher priority and needs review. labels Jan 1, 2024
Comment thread.github/workflows/cache_data.yaml Outdated
@seismanseisman removed the needs review This PR has higher priority and needs review. label Jan 2, 2024
@seisman
seisman merged commit 29f87b7 into mainJan 2, 2024
@seisman
seisman deleted the cache/earth_day branch January 2, 2024 00:32
@codspeed-hq

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #2937 will degrade performances by 6.45%

⚠️ No base runs were found

Falling back to comparing cache/earth_day (2ceb6eb) with main (29f87b7)

Summary

❌ 1 regressions
✅ 63 untouched benchmarks

⚠️Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmarkmaincache/earth_dayChange
test_grdlandmask_no_outgrid2.3 s2.4 s-6.45%

@seisman

Copy link
Copy Markdown
MemberAuthor

CodSpeed Performance Report

Merging #2937 will degrade performances by 6.45%

⚠️ No base runs were found

Falling back to comparing cache/earth_day (2ceb6eb) with main (29f87b7)

Summary

❌ 1 regressions ✅ 63 untouched benchmarks

⚠️Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark maincache/earth_day Change
test_grdlandmask_no_outgrid 2.3 s 2.4 s -6.45%

FYI, I've changed the regression threshold back to 10% to silence false warnings like the above:

image

@weiji14

Copy link
Copy Markdown
Member

FYI, I've changed the regression threshold back to 10% to silence false warnings like the above:

Thanks, I've opened an issue (#2942) to track why this benchmark test seems to have so many false warnings.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenanceBoring but important stuff for the core devs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@seisman@weiji14