✨ Add simple decay megacomplexes

[joernweissenborn]

This PR adds megacomplexes for sequential and parallel decay.

Change summary

The new megacomplexes are essentially syntactic sugar to simplify standard models. They look like this

megacomplex:
  mc1: 
    type: decay-sequential/decay-parallel,
      compartments: [s1, s2, s3],
      rates: [kinetic.1, kinetic.2, kinetic.3]

Checklist

• ✔️ Passing the tests (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

Closes issues

closes #XXXX

[github-actions]

👈 Launch a binder notebook on branch joernweissenborn/pyglotaran/feature/simpledecays

[codecov]

Codecov Report

Merging #860 (e9b1eb5) into main (91930ae) will increase coverage by 0.3%.
The diff coverage is 96.0%.

@@           Coverage Diff           @@
##            main    #860     +/-   ##
=======================================
+ Coverage   85.0%   85.4%   +0.3%     
=======================================
  Files         85      87      +2     
  Lines       4802    4874     +72     
  Branches     923     921      -2     
=======================================
+ Hits        4086    4163     +77     
+ Misses       563     559      -4     
+ Partials     153     152      -1     

Impacted Files	Coverage Δ
...n/builtin/megacomplexes/decay/decay_megacomplex.py	89.7% <88.2%> (+7.2%)	⬆️
glotaran/builtin/megacomplexes/decay/util.py	77.5% <91.1%> (+4.4%)	⬆️
glotaran/builtin/megacomplexes/decay/__init__.py	100.0% <100.0%> (ø)
.../megacomplexes/decay/decay_parallel_megacomplex.py	100.0% <100.0%> (ø)
...egacomplexes/decay/decay_sequential_megacomplex.py	100.0% <100.0%> (ø)
...iltin/megacomplexes/decay/initial_concentration.py	100.0% <100.0%> (ø)
glotaran/builtin/megacomplexes/decay/k_matrix.py	96.3% <100.0%> (+1.4%)	⬆️

---

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 91930ae...e9b1eb5. Read the comment docs.

[github-actions]

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

Benchmark diff v0.5.1 vs. main

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [96b42630]       [e9b1eb5a]
     <v0.5.1>                   
       73.9±0.8ms       70.8±0.4ms     0.96  BenchmarkOptimize.time_optimize(False, False, False)
         122±20ms          162±8ms    ~1.32  BenchmarkOptimize.time_optimize(False, False, True)
         71.2±1ms       72.3±0.7ms     1.02  BenchmarkOptimize.time_optimize(False, True, False)
          154±4ms         147±20ms     0.95  BenchmarkOptimize.time_optimize(False, True, True)
       90.7±0.4ms         88.9±3ms     0.98  BenchmarkOptimize.time_optimize(True, False, False)
        99.6±40ms        97.2±30ms     0.98  BenchmarkOptimize.time_optimize(True, False, True)
         90.9±2ms         88.8±1ms     0.98  BenchmarkOptimize.time_optimize(True, True, False)
         97.5±5ms         103±30ms     1.06  BenchmarkOptimize.time_optimize(True, True, True)
             203M             205M     1.01  IntegrationTwoDatasets.peakmem_optimize
       2.27±0.05s       2.12±0.06s     0.93  IntegrationTwoDatasets.time_optimize

Benchmark diff main vs. PR

Parametrized benchmark signatures:

BenchmarkOptimize.time_optimize(index_dependent, grouped, weight)

All benchmarks:

       before           after         ratio
     [91930ae8]       [e9b1eb5a]
         71.9±2ms       70.8±0.4ms     0.98  BenchmarkOptimize.time_optimize(False, False, False)
          159±7ms          162±8ms     1.02  BenchmarkOptimize.time_optimize(False, False, True)
         70.3±2ms       72.3±0.7ms     1.03  BenchmarkOptimize.time_optimize(False, True, False)
         154±20ms         147±20ms     0.96  BenchmarkOptimize.time_optimize(False, True, True)
         88.6±2ms         88.9±3ms     1.00  BenchmarkOptimize.time_optimize(True, False, False)
        97.6±30ms        97.2±30ms     1.00  BenchmarkOptimize.time_optimize(True, False, True)
         90.0±1ms         88.8±1ms     0.99  BenchmarkOptimize.time_optimize(True, True, False)
        97.7±30ms         103±30ms     1.05  BenchmarkOptimize.time_optimize(True, True, True)
             200M             205M     1.03  IntegrationTwoDatasets.peakmem_optimize
       2.29±0.04s       2.12±0.06s     0.92  IntegrationTwoDatasets.time_optimize

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
3.2% Duplication

[s-weigand]

At a first glimpse (didn't look at implementation details with the debugger, see failing comparison tests) there are mostly nitpicky things.

[s-weigand]

Codacy has valid complaints or is gamma a method for API consistency reasons?

[joernweissenborn]

Codacy has valid complaints or is gamma a method for API consistency reasons?

gamma is taken over from TIM(P). They call it like this there. Will be clarified when I refactor decay magacomplex to be in line with my thesis. For now it is just a "suck it up" function. Maybe @jsnel can give some context.

Still I should investigate if I cannot fix this complaints ;)

[s-weigand]

SonarCloud Quality Gate failed.

0 Bugs 0 Vulnerabilities 0 Security Hotspots 0 Code Smells

No Coverage information 25.1% Duplication

Most duplications are in the testes which should be gone with #866 (Ref.: #860 (comment)).

But I think something like this

        decay_megacomplexes = [
            m
            for m in dataset_model.megacomplex
            if isinstance(
                m, (DecayMegacomplex, DecayParallelMegacomplex, DecaySequentialMegacomplex)
            )
        ]

can be nicely factored out in a conveniences function filter_objects_by_type (...naming things😓) which we then could reuse all over the place.

[jsnel]

Codacy has valid complaints or is gamma a method for API consistency reasons?

gamma is taken over from TIM(P). They call it like this there. Will be clarified when I refactor decay magacomplex to be in line with my thesis. For now it is just a "suck it up" function. Maybe @jsnel can give some context.

This is not about the name gamma but its definition. In TIMP gamma is not even a function, let alone a method, it's just used as a vector.

Codacy correctly finds that:
"Method could be a function Used when a method doesn't use its bound instance, and so could be written as a function." (pylint no-self-use (R0201))

Just refactor it out or make it a staticmethod.

[jsnel]

The result consistency check is failing, with significant differences in a_matrix and decay_associated_spectra. This needs to be explained / investigated. If there was a (long standing) bug that was fixed then that bug must first be fixed independently on main.

[joernweissenborn]

The result consistency check is failing, with significant differences in a_matrix and decay_associated_spectra. This needs to be explained / investigated. If there was a (long standing) bug that was fixed then that bug must first be fixed independently on main.

I created a hotifx PR to backport the fix to 0.5.

[sourcery-ai]

Sourcery Code Quality Report

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

Quality metrics	Before	After	Change
Complexity	2.66 ⭐	3.08 ⭐	0.42 👎
Method Length	72.54 🙂	61.79 🙂	-10.75 👍
Working memory	9.64 🙂	9.35 🙂	-0.29 👍
Quality	66.38% 🙂	67.90% 🙂	1.52% 👍

Other metrics	Before	After	Change
Lines	839	851	12

Changed files	Quality Before	Quality After	Quality Change
glotaran/builtin/megacomplexes/decay/init.py	%	%	%
glotaran/builtin/megacomplexes/decay/decay_megacomplex.py	70.00% 🙂	91.10% ⭐	21.10% 👍
glotaran/builtin/megacomplexes/decay/initial_concentration.py	83.93% ⭐	88.17% ⭐	4.24% 👍
glotaran/builtin/megacomplexes/decay/k_matrix.py	78.25% ⭐	79.15% ⭐	0.90% 👍
glotaran/builtin/megacomplexes/decay/util.py	53.80% 🙂	55.09% 🙂	1.29% 👍
glotaran/builtin/megacomplexes/decay/test/test_decay_megacomplex.py	59.54% 🙂	59.54% 🙂	0.00%
glotaran/builtin/megacomplexes/decay/test/test_k_matrix.py	65.74% 🙂	71.83% 🙂	6.09% 👍

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

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
glotaran/builtin/megacomplexes/decay/util.py	calculate_decay_matrix_gaussian_irf	11 🙂	183 😞	18 ⛔	38.30% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/megacomplexes/decay/util.py	retrieve_decay_associated_data	1 ⭐	254 ⛔	17 ⛔	44.09% 😞	Try splitting into smaller methods. Extract out complex expressions
glotaran/builtin/megacomplexes/decay/util.py	finalize_data	15 🙂	105 🙂	13 😞	49.06% 😞	Extract out complex expressions
glotaran/builtin/megacomplexes/decay/test/test_decay_megacomplex.py	test_kinetic_model	3 ⭐	238 ⛔	9 🙂	54.99% 🙂	Try splitting into smaller methods
glotaran/builtin/megacomplexes/decay/util.py	retrieve_species_associated_data	6 ⭐	105 🙂	14 😞	55.78% 🙂	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!

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
31.4% Duplication

[s-weigand]

For the sake of getting on with the "merge train" I guess this is fine from my side.
Even so, I don't agree with the removing of DecayMegacomplexBase which added useless code duplication and cyclic import workarounds, just because of a disagreement about using abstract base classes

[jsnel]

Reviewed ok. Raised issue for the concerns voiced during review.

• ♻️ Address code duplication issues introduced in #860 #956