🩹Fix for crash when running optimization with more than 30 datasets #1184

jsnel · 2022-11-21T00:48:21Z

Fixes issue where when using more than 32 datasets the xarray operation .str.join across the dataset dimension would crash.

Change summary

🩹 Fix for crash when doing an optimization using more than 30 datasets

Checklist

✔️ Passing the tests (mandatory for all PR's)
🚧 Added changes to changelog (mandatory for all PR's)
👌 Closes issue (mandatory for ✨ feature and 🩹 bug fix PR's)

Closes issues

closes #1183

sourcery-ai · 2022-11-21T00:48:29Z

Sourcery Code Quality Report

❌ Merging this PR will decrease code quality in the affected files by 0.09%.

Quality metrics	Before	After	Change
Complexity	5.11 ⭐	5.10 ⭐	-0.01 👍
Method Length	52.61 ⭐	53.07 ⭐	0.46 👎
Working memory	8.89 🙂	8.89 🙂	0.00
Quality	68.17% 🙂	68.08% 🙂	-0.09% 👎

Other metrics	Before	After	Change
Lines	616	633	17

Changed files	Quality Before	Quality After	Quality Change
glotaran/optimization/data_provider.py	68.17% 🙂	68.08% 🙂	-0.09% 👎

Here are some functions in these files that still need a tune-up:

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
glotaran/optimization/data_provider.py	DataProvider.__init__	8 ⭐	229 ⛔	16 ⛔	39.60% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/optimization/data_provider.py	DataProviderLinked.align_weights	16 🙂	142 😞	10 😞	48.91% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/optimization/data_provider.py	DataProvider.add_model_weight	7 ⭐	149 😞	12 😞	52.50% 🙂	Try splitting into smaller methods. Extract out complex expressions
glotaran/optimization/data_provider.py	DataProviderLinked.align_groups	3 ⭐	120 😞	8 🙂	67.66% 🙂	Try splitting into smaller methods
glotaran/optimization/data_provider.py	DataProvider.get_axis_slice_from_interval	3 ⭐	82 🙂	10 😞	68.57% 🙂	Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

⭐ excellent
🙂 good
😞 poor
⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

github-actions · 2022-11-21T00:48:36Z

👈 Launch a binder notebook on branch jsnel/pyglotaran/fix/ufunc_with_more_than_32_operands

github-actions · 2022-11-21T00:52:30Z

Benchmark is done. Checkout the benchmark result page.
Benchmark differences below 5% might be due to CI noise.

Benchmark diff v0.6.0 vs. main

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [6c3c390e]       [83219f25]
     <v0.6.0>                   
!      46.0±0.4ms           failed      n/a  BenchmarkOptimize.time_optimize(False, False, False)
!      48.5±0.2ms           failed      n/a  BenchmarkOptimize.time_optimize(False, False, True)
!      46.1±0.2ms           failed      n/a  BenchmarkOptimize.time_optimize(False, True, False)
!      48.6±0.3ms           failed      n/a  BenchmarkOptimize.time_optimize(False, True, True)
!      56.6±0.6ms           failed      n/a  BenchmarkOptimize.time_optimize(True, False, False)
!       84.6±30ms           failed      n/a  BenchmarkOptimize.time_optimize(True, False, True)
!      56.1±0.2ms           failed      n/a  BenchmarkOptimize.time_optimize(True, True, False)
!       65.3±20ms           failed      n/a  BenchmarkOptimize.time_optimize(True, True, True)
             203M             205M     1.01  IntegrationTwoDatasets.peakmem_optimize
-      1.83±0.01s         986±20ms     0.54  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)


All benchmarks:

       before           after         ratio
     [83219f25]       [45a911f5]
           failed           failed      n/a  BenchmarkOptimize.time_optimize(False, False, False)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(False, False, True)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(False, True, False)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(False, True, True)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(True, False, False)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(True, False, True)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(True, True, False)
           failed           failed      n/a  BenchmarkOptimize.time_optimize(True, True, True)
             205M             205M     1.00  IntegrationTwoDatasets.peakmem_optimize
         986±20ms       1.01±0.04s     1.03  IntegrationTwoDatasets.time_optimize

codecov · 2022-11-21T00:55:03Z

Codecov Report

Base: 87.6% // Head: 87.6% // No change to project coverage 👍

Coverage data is based on head (83219f2) compared to base (83219f2).
Patch has no changes to coverable lines.

❗ Current head 83219f2 differs from pull request most recent head 45a911f. Consider uploading reports for the commit 45a911f to get more accurate results

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #1184   +/-   ##
=====================================
  Coverage   87.6%   87.6%           
=====================================
  Files        104     104           
  Lines       4946    4946           
  Branches     819     819           
=====================================
  Hits        4336    4336           
  Misses       493     493           
  Partials     117     117

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

joernweissenborn

lgtm

Fixes issue where when using more than 32 datasets the xarray operation .str.join across the dataset dimension would crash.

for more information, see https://pre-commit.ci

sonarqubecloud · 2022-11-21T01:09:09Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

…lotaran#1184) * 🩹Fix for ufunc with more than 32 operands * 'Refactored by Sourcery' * 📚 Add change to changelog * [pre-commit.ci] auto fixes from pre-commit.com hooks Co-authored-by: Sourcery AI <> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

sourcery-ai bot mentioned this pull request Nov 21, 2022

🩹Fix for ufunc with more than 32 operands (Sourcery refactored) #1185

Closed

jsnel requested a review from joernweissenborn November 21, 2022 00:57

joernweissenborn previously approved these changes Nov 21, 2022

View reviewed changes

jsnel changed the title ~~🩹Fix for ufunc with more than 32 operands~~ 🩹Fix for crash when running optimization with more than 30 datasets Nov 21, 2022

jsnel and others added 4 commits November 21, 2022 02:04

🩹Fix for ufunc with more than 32 operands

768e6cf

Fixes issue where when using more than 32 datasets the xarray operation .str.join across the dataset dimension would crash.

'Refactored by Sourcery'

bb44405

📚 Add change to changelog

f8af4df

[pre-commit.ci] auto fixes from pre-commit.com hooks

45a911f

for more information, see https://pre-commit.ci

jsnel dismissed joernweissenborn’s stale review via 45a911f November 21, 2022 01:08

jsnel force-pushed the fix/ufunc_with_more_than_32_operands branch from a7dd94a to 45a911f Compare November 21, 2022 01:08

jsnel requested a review from joernweissenborn November 21, 2022 01:09

joernweissenborn approved these changes Nov 21, 2022

View reviewed changes

jsnel merged commit a1bdb83 into glotaran:main Nov 21, 2022

jsnel deleted the fix/ufunc_with_more_than_32_operands branch November 21, 2022 01:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🩹Fix for crash when running optimization with more than 30 datasets #1184

🩹Fix for crash when running optimization with more than 30 datasets #1184

jsnel commented Nov 21, 2022 •

edited

Loading

sourcery-ai bot commented Nov 21, 2022 •

edited

Loading

github-actions bot commented Nov 21, 2022

github-actions bot commented Nov 21, 2022 •

edited

Loading

codecov bot commented Nov 21, 2022 •

edited

Loading

joernweissenborn left a comment

sonarqubecloud bot commented Nov 21, 2022

🩹Fix for crash when running optimization with more than 30 datasets #1184

🩹Fix for crash when running optimization with more than 30 datasets #1184

Conversation

jsnel commented Nov 21, 2022 • edited Loading

Change summary

Checklist

Closes issues

sourcery-ai bot commented Nov 21, 2022 • edited Loading

Sourcery Code Quality Report

Legend and Explanation

github-actions bot commented Nov 21, 2022

github-actions bot commented Nov 21, 2022 • edited Loading

codecov bot commented Nov 21, 2022 • edited Loading

Codecov Report

joernweissenborn left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 21, 2022

jsnel commented Nov 21, 2022 •

edited

Loading

sourcery-ai bot commented Nov 21, 2022 •

edited

Loading

github-actions bot commented Nov 21, 2022 •

edited

Loading

codecov bot commented Nov 21, 2022 •

edited

Loading