search

lincc-frameworks / tape

[Deprecated] Package for working with LSST time series data
https://tape.readthedocs.io
MIT License
12 stars 3 forks source link

Pin dask-expr to < 1.10.11 and revert "Add support for running batch on a single lightcurve" #433

Closed wilsonbb closed 4 months ago

wilsonbb commented 4 months ago

Reverts lincc-frameworks/tape#420 and pins dask-expr <1.0.11 to address flakiness reported in https://github.com/lincc-frameworks/tape/issues/434. The plan is to do another PR for the batch single_lc changes in #420 after all of the batch-related flakiness has been addressed.

Note that even with these changes we're still seeing the test_batch_by_band errors referenced in lincc-frameworks/tape#434 consistently on the python 3.11.9 build, but this is still an improvement over the current test failures we've been seeing in https://github.com/lincc-frameworks/tape/issues/427

review-notebook-app[bot] commented 4 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

github-actions[bot] commented 4 months ago
Before [89aa1169] After [49c8c480] Ratio Benchmark (Parameter)
33.7±0.2ms 34.2±0.3ms 1.02 benchmarks.time_prune_sync_workflow
33.6±0.3ms 33.2±0.4ms 0.99 benchmarks.time_batch

Click here to view all benchmarks.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 95.82%. Comparing base (89aa116) to head (95fb692).

Files Patch % Lines
src/tape/ensemble.py 77.77% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #433 +/- ## ========================================== - Coverage 95.84% 95.82% -0.03% ========================================== Files 25 25 Lines 1781 1771 -10 ========================================== - Hits 1707 1697 -10 Misses 74 74 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wilsonbb commented 4 months ago

Thank you for looking into this. Did you try playing with the dask version itself at all? Their test suite is failing atm, and I wonder if pinned dask itself to something older would stabilize things

I've messed around a bit, but among the compatible dask versions, I didn't see the error disappear. But probably worth checking again