Skip to content
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

🩹 Transform standard error if parameter has non_negative constraint #1320

Merged

Conversation

jsnel
Copy link
Member

@jsnel jsnel commented Jul 9, 2023

Correctly report standard error in the case that non_negative contraint is used.

Change summary

  • 👌Improve reporting of standard error in case of non_negative constraint in the parameter

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)
  • 🧪 Adds new tests for the feature (mandatory for ✨ feature and 🩹 bug fix PR's)
  • 📚 Adds documentation of the feature

@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2023

Binder 👈 Launch a binder notebook on branch jsnel/pyglotaran/fix/correct_t_values_with_non_negative_params

@s-weigand s-weigand force-pushed the fix/correct_t_values_with_non_negative_params branch from 364071a to 8c81edb Compare July 9, 2023 21:48
@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2023

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

Benchmark diff v0.7.0 vs. main

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [9adf4a49]       [1e583f11]
     <v0.7.0>                   
           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)
             216M             219M     1.01  IntegrationTwoDatasets.peakmem_optimize
       1.60±0.04s       1.58±0.08s     0.98  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [1e583f11]       [2a278b6c]
           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)
             219M             221M     1.01  IntegrationTwoDatasets.peakmem_optimize
       1.58±0.08s       1.58±0.03s     1.00  IntegrationTwoDatasets.time_optimize

@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Patch coverage: 71.4% and project coverage change: -0.1 ⚠️

Comparison is base (1e583f1) 88.5% compared to head (2a278b6) 88.5%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1320     +/-   ##
=======================================
- Coverage   88.5%   88.5%   -0.1%     
=======================================
  Files        107     107             
  Lines       5122    5128      +6     
  Branches     960     962      +2     
=======================================
+ Hits        4538    4542      +4     
- Misses       470     471      +1     
- Partials     114     115      +1     
Impacted Files Coverage Δ
glotaran/optimization/optimizer.py 80.8% <71.4%> (-0.8%) ⬇️

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

@jsnel jsnel requested review from s-weigand and ism200 July 9, 2023 22:16
@jsnel jsnel marked this pull request as ready for review July 14, 2023 22:40
@jsnel jsnel force-pushed the fix/correct_t_values_with_non_negative_params branch from 3dd9334 to 2a278b6 Compare July 20, 2023 14:08
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@ism200 ism200 left a comment

Choose a reason for hiding this comment

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

thanks

Copy link
Member

@s-weigand s-weigand left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jsnel jsnel merged commit e169668 into glotaran:main Jul 21, 2023
@jsnel jsnel deleted the fix/correct_t_values_with_non_negative_params branch July 21, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants