fix: make SLH scattering field concretely typed#34
Open
oameye wants to merge 3 commits into
Open
Conversation
SMatrix carries a hidden length parameter L, so declaring the field as
SMatrix{N,N,ST} left L free and rendered the field abstract. Add L to
the struct and an inner constructor that infers it, preserving the
existing SLH{N,ST,LT,HT}(...) calling convention.
There was a problem hiding this comment.
Benchmark Results
Details
| Benchmark suite | Current: 7bdde5f | Previous: 6a468fd | Ratio |
|---|---|---|---|
Correlations/two-time/single photon cavity |
183182452 ns |
183117800 ns |
1.00 |
Interaction Picture/coefficient matrix M/analytical (2 equal modes) |
13355 ns |
13104 ns |
1.02 |
Interaction Picture/coefficient matrix M/numerical (ODE) |
73768 ns |
73197 ns |
1.01 |
Interaction Picture/coupling matrix evaluation/2 modes |
95.8377239199157 ns |
96.30126849894292 ns |
1.00 |
Interaction Picture/coupling matrix evaluation/4 modes |
202.41472868217053 ns |
204.60943396226415 ns |
0.99 |
Interaction Picture/operator substitution/TLS cascade |
17703 ns |
17773 ns |
1.00 |
Pulse Couplings/multi-pulse/input 2 modes |
415798 ns |
414315 ns |
1.00 |
Pulse Couplings/multi-pulse/output 2 modes |
463096 ns |
464319 ns |
1.00 |
Pulse Couplings/single-pulse/input |
368309 ns |
369711 ns |
1.00 |
Pulse Couplings/single-pulse/input Gaussian |
89.03866248693835 ns |
89.57306889352819 ns |
0.99 |
Pulse Couplings/single-pulse/output |
401572 ns |
403004 ns |
1.00 |
Pulse Couplings/single-pulse/output Gaussian |
92.51048218029351 ns |
90.81799163179916 ns |
1.02 |
SLH Algebra/closure evaluation/2-QD waveguide H(t) |
35567 ns |
34375 ns |
1.03 |
SLH Algebra/closure evaluation/2-QD waveguide L(t) |
3363.875 ns |
3233.5 ns |
1.04 |
SLH Algebra/numeric/2-QD waveguide composition |
30627 ns |
21570 ns |
1.42 |
SLH Algebra/symbolic/3-cavity cascade |
6858.8 ns |
7576.75 ns |
0.91 |
SLH Algebra/symbolic/concatenate + cascade |
2166.222222222222 ns |
2702.777777777778 ns |
0.80 |
SLH Algebra/symbolic/feedback OPO loop |
5867 ns |
6590.2 ns |
0.89 |
Translation/closure evaluation/3-cavity H(t) |
8673 ns |
8616 ns |
1.01 |
Translation/closure evaluation/3-cavity L(t) |
8656 ns |
9077 ns |
0.95 |
Translation/static/atom-cavity |
40406 ns |
39484 ns |
1.02 |
Translation/time-dependent/3-cavity H+L |
616654 ns |
596616 ns |
1.03 |
Translation/time-dependent/atom-cavity |
134461 ns |
122599 ns |
1.10 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Benchmark Results'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.
| Benchmark suite | Current: 7bdde5f | Previous: 6a468fd | Ratio |
|---|---|---|---|
SLH Algebra/numeric/2-QD waveguide composition |
30627 ns |
21570 ns |
1.42 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
==========================================
- Coverage 96.78% 96.77% -0.01%
==========================================
Files 4 4
Lines 404 403 -1
==========================================
- Hits 391 390 -1
Misses 13 13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SMatrixcarries a hidden length parameterL(the total element countN*N), so declaring the field asSMatrix{N,N,ST}leftLfree and rendered thescatteringfield abstract. This was caught by theCheckConcreteStructs(Concretely typed) test set.Adds
Lto the struct and an inner constructor that infers it from the matrix, preserving the existingSLH{N,ST,LT,HT}(...)calling convention so no call sites needed changes. Existing dispatch sites (SLH{N},SLH{N,ST,LT,HT}) are partial type applications and still match.