Skip to content

SYNC on gsp/Forecast/all and gsp/{gsp_id}/forecast - DRAFT#255

Merged
peterdudfield merged 15 commits into
fix/forecast-all-reduce-cpufrom
forecast-all-sync
Mar 30, 2026
Merged

SYNC on gsp/Forecast/all and gsp/{gsp_id}/forecast - DRAFT#255
peterdudfield merged 15 commits into
fix/forecast-all-reduce-cpufrom
forecast-all-sync

Conversation

@peterdudfield

@peterdudfield peterdudfield commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

Building on from #250 with a few other fixes.

  • change forecast/all to use gprc-requests
  • chagne forecast/ 1 to use gprc-requests
  • include jinga2
  • put back in forecast/all for one timestamp

I wanted to compare the cpu usages and performance compared to #252

Q: Do we need to cache on forecast/all down to 1 hour, but leave it as 1 day on the background_task bit?
Q: Show we just move all routes to use grpc-request?

Helps with https://github.com/openclimatefix/client-private/issues/294

How Has This Been Tested?

  • Deployed on dev on 2026-03-29 15:30 ish

  • Deployed change on gsp/{gsp_id}/forecast at 2026-03-29 17:50 ish

  • CPU seems to stay below 20% (although start up it was quite big)

  • UI all working

  • checks in airflow all working, and much faster

Screenshot 2026-03-29 at 20 34 04 Screenshot 2026-03-30 at 07 22 46

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@peterdudfield peterdudfield changed the title SYNC on Forecast/all - DRAFT SYNC on gsp/Forecast/all and gsp/{gsp_id}/forecast - DRAFT Mar 29, 2026
@peterdudfield peterdudfield mentioned this pull request Mar 29, 2026
9 tasks
@braddf braddf changed the base branch from main to fix/forecast-all-reduce-cpu March 30, 2026 12:25

@braddf braddf left a comment

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.

Not sure about the jinja2 addition but otherwise think makes sense so will merge in 👍

@braddf braddf changed the base branch from fix/forecast-all-reduce-cpu to fix/forecast-all-reduce-cpu-pd March 30, 2026 12:47
@braddf braddf changed the base branch from fix/forecast-all-reduce-cpu-pd to fix/forecast-all-reduce-cpu March 30, 2026 12:47
@peterdudfield peterdudfield merged commit b3d8011 into fix/forecast-all-reduce-cpu Mar 30, 2026
8 checks passed
@peterdudfield peterdudfield deleted the forecast-all-sync branch March 30, 2026 13:30
@peterdudfield peterdudfield mentioned this pull request Mar 30, 2026
5 tasks
braddf added a commit that referenced this pull request Mar 30, 2026
… CPU (#250)

* fix(forecast-all): swap json encoding to use Rust-based adapter

* fix(forecast-all): PoC grpc-requests sync client

* chore(forecast-all): ruff ignores

* chore(forecast-all): lint

* chore(uv): fix uv.lock merge issue

* SYNC on gsp/Forecast/all and gsp/{gsp_id}/forecast - DRAFT (#255)

* remake uv.lock, remove strip and timezone

* lint

* add jinja2

* TDD: add tests

* dont use cache if start and end is the same

* add params back in

* add jinja2

* lint

* also use sync for `GetForecastAsTimeseries`

* remove code and tidy

* dont use real host or port

* lint

* add mocking

* fix(forecast-all): add timestamp validation when gsp_ids not set

* chore(forecast-all): lint fixes

* fix(forecast-all): use default factories for start/end

* fix(forecast-all): sort gsp compact=false response

---------

Co-authored-by: Peter Dudfield <34686298+peterdudfield@users.noreply.github.com>
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