-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch from using prophet-prebuilt to prophet #4045
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4045 +/- ##
=======================================
- Coverage 99.7% 99.7% -0.0%
=======================================
Files 349 349
Lines 37546 37548 +2
=======================================
+ Hits 37428 37429 +1
- Misses 118 119 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
is_using_windows, | ||
): | ||
if is_using_windows: | ||
pytest.xfail( | ||
"Pipeline code generation is not expected to work on Windows machines", | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -116,7 +116,7 @@ def get_prediction_intervals( | |||
y: Optional[pd.Series] = None, | |||
coverage: List[float] = None, | |||
) -> Dict[str, pd.Series]: | |||
"""Find the prediction intervals using the fitted ProphetRegressor. | |||
"""Find the prediction intervals using the fitted XGBoostRegressor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin would be proud
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When testing in tempojobs, I had to do a small tweak for M1s (I believe it's limited to them). All tests passed
python -m pip install .[test] | ||
python -m pip install .[prophet] | ||
pip freeze | ||
- if: ${{ matrix.command != 'git-test-prophet' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For linux workflows, we also run git-test-automl
with prophet installed. I decided not to do that for windows because of known slowness when using the pip installation of prophet on windows - the tests were taking over an hour
pytest evalml/tests/component_tests/test_prophet_regressor.py evalml/tests/component_tests/test_components.py evalml/tests/component_tests/test_utils.py evalml/tests/pipeline_tests/ evalml/tests/utils_tests/ -n 2 --durations 0 --timeout $(TIMEOUT) --cov=evalml --cov-config=./pyproject.toml --junitxml=test-reports/git-test-prophet-junit.xml | ||
pytest evalml/tests/component_tests/test_prophet_regressor.py evalml/tests/component_tests/test_components.py evalml/tests/component_tests/test_utils.py evalml/tests/pipeline_tests/test_pipelines.py -n 2 --durations 0 --timeout $(TIMEOUT) --cov=evalml --cov-config=./pyproject.toml --junitxml=test-reports/git-test-prophet-junit.xml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved these tests from git-test-prophet
to git-test-other
since they don't actually need prophet installed in order to run. This sped up the windows prophet test times from 45 minutes to 21, and only increased the windows other test times from 13 minutes to 16.
Closes #3908