Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

🐛 check for None before setting n_jobs to nb logical CPUs #126

Merged
merged 8 commits into from
May 15, 2024

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented May 6, 2024

Following the merger of #118 a bug slipped into tsflex.

Parsing the n_jobs ignored user-specified values (i.e. integers) and always selected the number of logical cores. As such, it was impossible for users to manage the number of jobs...

@jvdd jvdd requested a review from jonasvdd May 6, 2024 12:03
@jonasvdd jonasvdd self-assigned this May 6, 2024
@jonasvdd jonasvdd added the bug Something isn't working label May 6, 2024
Copy link

codspeed-hq bot commented May 6, 2024

CodSpeed Performance Report

Merging #126 will degrade performances by 97.95%

Comparing bugfix_parse_njobs (bf1ca48) with main (577e4c2)

Summary

❌ 2 regressions
✅ 379 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main bugfix_parse_njobs Change
test_single_series_feature_collection_multiple_descriptors[1] 483.5 ms 23,548.8 ms -97.95%
test_single_series_feature_collection_multiple_descriptors[2] 401.2 ms 1,033.1 ms -61.17%

@jonasvdd
Copy link
Member

The tsfel LPCC - bug - for which we provided a hotfix is fixed in tsfel>=0.1.4

https://github.com/fraunhoferportugal/tsfel/releases/tag/v0.1.4

@jonasvdd
Copy link
Member

Ready for review @jvdd!

Copy link
Member Author

@jvdd jvdd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jvdd jvdd merged commit e12e550 into main May 15, 2024
19 checks passed
@jvdd jvdd deleted the bugfix_parse_njobs branch August 24, 2024 10:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants