Skip to content

feat(proto): Add p2,98,25,75; refactor tests#181

Open
devsjc wants to merge 1 commit into
mainfrom
devsjc/more-plevels
Open

feat(proto): Add p2,98,25,75; refactor tests#181
devsjc wants to merge 1 commit into
mainfrom
devsjc/more-plevels

Conversation

@devsjc

@devsjc devsjc commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Adds 4 more p-levels to the predicted generation values table. These cost nothing storage-wise if they are unused. Also refactors the test suite a little bit.

Contribution Checklist

  • Have you followed the Open Climate Fix Contribution Guidelines?
  • Have you referenced the Issue this PR addresses, where applicable?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added a summary of the changes?
  • Have you written new tests for your changes, where applicable?
  • Have you successfully run make lint with your changes locally?
  • Have you successfully run make test with your changes locally?

Warning

PRs may be closed if all the above boxes are not checked.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Benchmark Results

Benchmark results
?   	github.com/openclimatefix/data-platform/cmd	[no test files]
?   	github.com/openclimatefix/data-platform/internal/gen/ocf/dp	[no test files]
?   	github.com/openclimatefix/data-platform/internal/interceptors	[no test files]
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/dummy	0.005s
{"level":"debug","time":"2026-07-01T09:21:10Z","message":"Completed migrations"}
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 9V74 80-Core Processor                
BenchmarkPostgresClient/small/GetForecastAsTimeseries-4         	      50	  22423381 ns/op
BenchmarkPostgresClient/small/GetForecastAtTimestamp-4          	     195	   6025208 ns/op
BenchmarkPostgresClient/small/GetObservationsAsTimeseries-4     	     979	   1143904 ns/op
BenchmarkPostgresClient/small/CreateForecast-4                  	     128	   9086413 ns/op
BenchmarkPostgresClient/small/StreamForecastData-4              	      21	  51320770 ns/op
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/postgres	60.497s
?   	github.com/openclimatefix/data-platform/internal/server/postgres/gen	[no test files]
Benchmark vs base branch
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 9V74 80-Core Processor                
                                                   │ bench-main.txt │    bench-devsjc-more-plevels.txt     │
                                                   │     sec/op     │    sec/op     vs base                │
PostgresClient/small/GetForecastAsTimeseries-4         22.11m ± ∞ ¹   22.42m ± ∞ ¹       ~ (p=1.000 n=1) ²
PostgresClient/small/GetForecastAtTimestamp-4          5.758m ± ∞ ¹   6.025m ± ∞ ¹       ~ (p=1.000 n=1) ²
PostgresClient/small/GetObservationsAsTimeseries-4     1.154m ± ∞ ¹   1.144m ± ∞ ¹       ~ (p=1.000 n=1) ²
PostgresClient/small/CreateForecast-4                  9.036m ± ∞ ¹   9.086m ± ∞ ¹       ~ (p=1.000 n=1) ²
PostgresClient/small/StreamForecastData-4              52.85m ± ∞ ¹   51.32m ± ∞ ¹       ~ (p=1.000 n=1) ²
geomean                                                9.316m         9.366m        +0.53%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

@devsjc devsjc linked an issue Jul 1, 2026 that may be closed by this pull request
3 tasks
(buf.validate.field).map.max_pairs = 20,
(buf.validate.field).map.keys = {
string: {in: ["p10", "p90"]}
string: {in: ["p10", "p90", "p2", "p98", "p25", "p75"]}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you order this?

DROP COLUMN p2_sip,
DROP COLUMN p98_sip,
DROP COLUMN p25_sip,
DROP COLUMN p75_sip;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

perhaps order these p2, p25, p75, p98, just to make it easier to read.

I know it doesnt make any different in the sql tables

GeometryUuid: uuid.MustParse(req.LocationUuid),
SourceTypeID: int16(req.EnergySource.Number()),
AtTimestampUtc: pgtype.Timestamp{Time: req.ValidFromUtc.AsTime(), Valid: true},
AtTimestampUtc: pgtype.Timestamp{Time: validFrom, Valid: true},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice

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.

Cant update location if valid_from_utc not set

2 participants