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

Feature: Damped Oscillation Megacomplex #764

Merged
merged 9 commits into from
Aug 15, 2021

Conversation

joernweissenborn
Copy link
Member

@joernweissenborn joernweissenborn commented Aug 8, 2021

This PR adds the DampedOscillationMegacomplex model to pyglotaran.

Change summary

  • Add DampedOscillationMegacomplex as type damped-oscillation
  • Small refactor in analysis.util and builtin/megacomplex/decay

Checklist

  • ✔️ Passing the tests (mandatory for all PR's)
  • 🧪 Adds new tests for the feature (mandatory for ✨ feature and 🩹 bug fix PR's)

@joernweissenborn joernweissenborn requested review from jsnel, s-weigand and a team as code owners August 8, 2021 14:10
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2021

Binder 👈 Launch a binder notebook on branch joernweissenborn/pyglotaran/feature/damped_osc_megacmplx

@joernweissenborn joernweissenborn changed the base branch from main to staging August 8, 2021 14:11
@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #764 (239df81) into staging (d2b297b) will increase coverage by 0.0%.
The diff coverage is 86.2%.

Impacted file tree graph

@@           Coverage Diff           @@
##           staging    #764   +/-   ##
=======================================
  Coverage     83.9%   83.9%           
=======================================
  Files           73      75    +2     
  Lines         4089    4185   +96     
  Branches       736     752   +16     
=======================================
+ Hits          3431    3512   +81     
- Misses         528     538   +10     
- Partials       130     135    +5     
Impacted Files Coverage Δ
...n/builtin/megacomplexes/decay/decay_megacomplex.py 82.5% <ø> (-0.3%) ⬇️
...mped_oscillation/damped_oscillation_megacomplex.py 83.5% <83.5%> (ø)
glotaran/analysis/util.py 90.9% <100.0%> (+<0.1%) ⬆️
...iltin/megacomplexes/damped_oscillation/__init__.py 100.0% <100.0%> (ø)
glotaran/builtin/megacomplexes/decay/irf.py 66.2% <100.0%> (+1.9%) ⬆️
glotaran/builtin/megacomplexes/decay/util.py 70.3% <100.0%> (ø)
glotaran/model/model.py 85.6% <100.0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2b297b...239df81. Read the comment docs.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 8, 2021

This pull request fixes 3 alerts when merging a6c90f7 into 5efcf0c - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 1 for Comparison using is when operands support `__eq__`

@s-weigand s-weigand force-pushed the feature/damped_osc_megacmplx branch from b4570f7 to 84c6877 Compare August 12, 2021 19:53
jsnel added a commit to glotaran/pyglotaran-examples that referenced this pull request Aug 13, 2021
Add a basic example to test doas feature branch, see PR pyglotaran#764: glotaran/pyglotaran#764
jsnel added a commit to glotaran/pyglotaran-examples that referenced this pull request Aug 13, 2021
Add a basic example to test doas feature branch, see PR pyglotaran#764: glotaran/pyglotaran#764
@jsnel jsnel force-pushed the feature/damped_osc_megacmplx branch from 84c6877 to 6967ba1 Compare August 13, 2021 21:38
@jsnel jsnel force-pushed the staging branch 2 times, most recently from 3f663cf to d2b297b Compare August 14, 2021 10:12
@jsnel jsnel force-pushed the feature/damped_osc_megacmplx branch from 6967ba1 to bb9a2f7 Compare August 14, 2021 12:34
@sonarcloud
Copy link

sonarcloud bot commented Aug 15, 2021

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

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Aug 15, 2021

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 3.00%.

Quality metrics Before After Change
Complexity 4.78 ⭐ 3.43 ⭐ -1.35 👍
Method Length 71.62 🙂 70.93 🙂 -0.69 👍
Working memory 9.52 🙂 9.13 🙂 -0.39 👍
Quality 64.39% 🙂 67.39% 🙂 3.00% 👍
Other metrics Before After Change
Lines 585 606 21
Changed files Quality Before Quality After Quality Change
glotaran/analysis/util.py 62.73% 🙂 67.90% 🙂 5.17% 👍
glotaran/analysis/test/models.py 66.10% 🙂 66.88% 🙂 0.78% 👍

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

File Function Complexity Length Working Memory Quality Recommendation
glotaran/analysis/util.py apply_relations 8 ⭐ 121 😞 10 😞 58.41% 🙂 Try splitting into smaller methods. Extract out complex expressions
glotaran/analysis/util.py find_overlap 8 ⭐ 71 🙂 13 😞 60.15% 🙂 Extract out complex expressions
glotaran/analysis/util.py retrieve_clps 7 ⭐ 104 🙂 10 😞 61.56% 🙂 Extract out complex expressions
glotaran/analysis/util.py calculate_matrix 8 ⭐ 80 🙂 10 😞 64.01% 🙂 Extract out complex expressions
glotaran/analysis/util.py combine_matrix 5 ⭐ 82 🙂 10 😞 66.60% 🙂 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!

Copy link
Member

@jsnel jsnel left a comment

Choose a reason for hiding this comment

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

Reviewed ok, tested against the beta doas example found here at the time of the review.

@jsnel jsnel changed the title Feature: Damped Oscillation Megacmomplex Feature: Damped Oscillation Megacomplex Aug 15, 2021
@github-actions
Copy link
Contributor

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

Benchmark diff

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [dc00e6da]       [239df816]
     <v0.4.0>                   
-      57.1±0.3ms       40.8±0.5ms     0.71  BenchmarkOptimize.time_optimize(False, False, False)
-         324±4ms         52.0±1ms     0.16  BenchmarkOptimize.time_optimize(False, False, True)
-        82.3±1ms       70.1±0.2ms     0.85  BenchmarkOptimize.time_optimize(False, True, False)
         85.4±1ms         81.3±1ms     0.95  BenchmarkOptimize.time_optimize(False, True, True)
       57.9±0.9ms       55.2±0.5ms     0.95  BenchmarkOptimize.time_optimize(True, False, False)
-         323±3ms        68.1±20ms     0.21  BenchmarkOptimize.time_optimize(True, False, True)
         83.2±1ms         89.5±3ms     1.08  BenchmarkOptimize.time_optimize(True, True, False)
         85.0±2ms         96.8±3ms    ~1.14  BenchmarkOptimize.time_optimize(True, True, True)
             182M             181M     0.99  IntegrationTwoDatasets.peakmem_create_result
             197M             198M     1.00  IntegrationTwoDatasets.peakmem_optimize
          268±4ms          211±4ms    ~0.79  IntegrationTwoDatasets.time_create_result
-         5.45±0s       1.68±0.05s     0.31  IntegrationTwoDatasets.time_optimize

jsnel added a commit that referenced this pull request Sep 16, 2021
* Added DampedOscillationMegacomplex
* Added damped-oscillation to setup.cfg plugins

Co-authored-by: Joris Snellenburg <[email protected]>
s-weigand added a commit to glotaran/pyglotaran-examples that referenced this pull request Sep 17, 2021
* Basic DOAS example (beta)

Add a basic example to test doas feature branch, see PR pyglotaran#764: glotaran/pyglotaran#764

* Test doas example on CI

* 🩹 Added doas data  and made an exception in the .gitignore

* 🧹 Removed old style 'type' in the model

* 👌 Added coherent artifact for testing

'vary: False' and 'maximum_number_function_evaluations=1' are only set for performance reasons

* ✨Added data file with more values

Co-authored-by: s-weigand <[email protected]>
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.

2 participants