Return meaningful results from the Train-Predict pipeline compute method#1822
Return meaningful results from the Train-Predict pipeline compute method#1822
Conversation
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ensorAPI if model passed in as field in api call Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
… created Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…edictPipeline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…Pipeline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ictPipeline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ictPipeline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…eline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…es in TrainPredictPipeline Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Documentation build overview
Show files changed (3 files in total): 📝 3 modified | ➕ 0 added | ➖ 0 deleted
|
…is to fix error when running forecaster data generator it returns only job id's when ran as_job as forecasts haven't been generated yet. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ning as job Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com>
…wer measurement to True Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…tion. to do will be simplified Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #1811 by implementing meaningful return values for the TrainPredictPipeline.compute() method, which previously always returned an empty list. The changes introduce different return formats based on execution mode: forecast data with sensor information for synchronous execution (as_job=False), and job IDs for asynchronous execution (as_job=True).
Key changes:
- Added
return_valuesinstance variable to collect and return results from pipeline execution - Modified
run_cycle()to append forecast data and sensor information to return values - Updated
run()method to return job IDs when executing asynchronously - Extended test assertions to validate the return value structure and content
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| flexmeasures/data/models/forecasting/pipelines/train_predict.py | Implements return value collection and handling for both synchronous and asynchronous execution modes |
| flexmeasures/data/models/forecasting/pipelines/predict.py | Returns the BeliefsDataFrame from the predict pipeline run |
| flexmeasures/data/models/forecasting/init.py | Conditionally skips event resolution validation when running as job |
| flexmeasures/data/models/data_sources.py | Conditionally skips sensor and source assignment when running as job |
| flexmeasures/data/tests/test_train_predict_pipeline.py | Adds comprehensive assertions to validate return values for both execution modes |
| flexmeasures/api/v3_0/sensors.py | Removes extra blank line (whitespace cleanup) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flexmeasures/data/models/forecasting/pipelines/train_predict.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com>
Flix6x
left a comment
There was a problem hiding this comment.
Very nice. I believe I only have rather small suggestions.
|
One more note: if the response message in the forecasting CLI command incorporates this new feature, then this PR by itself also becomes a user-facing feature. And it's useful to consider that when writing the changelog entry (i.e. from a user perspective, how does this PR help them). |
Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com> Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…error Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…peline test Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
- Reports number of forecast jobs created when --as-job is used - Reports number of forecast beliefs and unique belief times for direct computation - Displays error message if no forecasts or jobs were create Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
…ature and cli response update Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Thanks for the review, i applied the suggestions and added the changelog entry. |
Flix6x
left a comment
There was a problem hiding this comment.
Thanks. Just the main changelog entry still.
…alues feature and cli response update" This reverts commit c29c870.
… for `flexmeasures add forecasts` CLI command Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Flix6x
left a comment
There was a problem hiding this comment.
Just one small formatting issue. Feel free to merge afterwards.
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Description
This pr aims to complete the return values of the
TrainPredictPipeline.compute()TrainPredictPipeline.compute()always returned an empty list.as_job = False, the pipeline now returns the computed forecast results. in format:{ "data": <BeliefsDataFrame of forecasts>, "sensor": <Sensor object the forecasts are stored on> }as_job = True, the pipeline returns a list of job IDs ["job-1": , "job-2": ].[ {"job-1": <job_id_1>}, {"job-2": <job_id_2>}, ... ]CLI improvements:
flexmeasures add forecastscommand now reports detailed success messages.--as-jobis set, it shows the number of forecast jobs created.How to test:
You can now run:
pytest flexmeasures/data/tests/test_train_predict_pipeline.py.I added assertions to validate the return values of the pipelines, so the tests now cover both the as_job=False and as_job=True cases.
Related Items
closes #1811
...
Sign-off