Added Random Forest Regressor as an additional prediction model. - #12767
Conversation
|
Implemented RandomForestRegressor alongside SVR and SARIMAX |
for more information, see https://pre-commit.ci
|
Added Random Forest Regressor to main voting |
|
Hi @poyea, |
|
Hi @TheAlgorithms, |
Used matplotlib to plot actual vs predicted user count, forecast confidence intervals, outlier thresholds from IQR. Added logging instead of print because in production, print() is not scalable.
for more information, see https://pre-commit.ci
|
Hi @TheAlgorithms, |
There was a problem hiding this comment.
Pull request overview
This PR extends the existing forecasting “voting” script by adding a Random Forest–based regressor as an additional prediction option, and introduces a small visualization helper for comparing predictions. It also updates the repository’s directory index.
Changes:
- Add
random_forest_regressor()(sklearn) and include it in theres_voteensemble. - Add
plot_forecast()(matplotlib) and call it from the__main__block to visualize results. - Update
DIRECTORY.mdto add a Geometry index entry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| machine_learning/forecasting/run.py | Adds RandomForestRegressor forecasting method and a matplotlib plotting helper; integrates both into the main execution path. |
| DIRECTORY.md | Adds a Geometry section index entry (“Segment Intersection”). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@priya-sundaram-dev We already have the file machine_learning/random_forest_regressor.py Does it add anything to have another implementation? |
|
@algorithms-keeper review-all |
Click here to look at the relevant links ⬇️
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
|
|
Thanks @cclauss. Distinguishing the two: The regressor function itself is clean and its doctest is deterministic — I ran it locally and Two things I'd ask for before merge, though:
So: not a duplicate, but please strip the unused plotting + matplotlib import, then I think it's a reasonable incremental add. Happy to re-review. |
|
@priya-sundaram-dev Can you please add one or more GitHub suggestions that enable us to drop plot_forecast and its imports? |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Thanks for adding the RF regressor — the random_forest_regressor doctest is deterministic (random_state=42 → 1.95) and it's a genuine 4th voter, so that part is good to merge. The only blocker is the plotting: plot_forecast is unused dead code, has no type hints/doctest, and is the sole reason matplotlib gets imported (an extra heavy dependency pulled in at import time for a function that only runs an interactive plt.show()). Here are three suggestions to drop it cleanly — accepting all three keeps the RF addition and removes only the plotting:
Co-authored-by: priya-sundaram-dev <oc-409d01@agentmail.to>
Describe your change:
Implemented RandomForestRegressor alongside SVR and SARIMAX
Checklist: