Skip to content

Remove unnecessary Option from assembly::fragments#151

Merged
Garrett-Pz merged 1 commit into
mainfrom
sure-fragmentation
Jun 16, 2026
Merged

Remove unnecessary Option from assembly::fragments#151
Garrett-Pz merged 1 commit into
mainfrom
sure-fragmentation

Conversation

@jdaymude

@jdaymude jdaymude commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

This PR drops the Option<_> part of assembly::fragments's return type and the corresponding Some/None check in assembly::recurse_index_search.

In #121, we updated match enumeration to use the DAG approach from Seet et al. (2025). Specifically, we implemented matches::matches_to_remove which uses the DAG to determine the list of matches that can be removed from a given assembly state. Thus, it is now impossible for assembly::fragments to ever return None, as this would indicate a match cannot be found among an assembly state's fragments.

Benchmarks show this is a performance-neutral change; the rare improvements/regressions seem to be noise.

Benchmark b8305c8 (this PR) vs. e7ca3d4 (current main) as a baseline
bench_matches/gdb13_1201/nauty                        
                        time:   [55.221 ms 55.234 ms 55.247 ms]
                        change: [+0.4991% +0.5793% +0.6588%] (p = 0.00 < 0.05)
                        Change within noise threshold.
bench_matches/gdb13_1201/tree-nauty                        
                        time:   [34.729 ms 34.765 ms 34.811 ms]
                        change: [+2.9248% +3.0520% +3.1796%] (p = 0.00 < 0.05)
                        Performance has regressed.
bench_matches/gdb17_200/nauty                        
                        time:   [203.14 ms 203.20 ms 203.27 ms]
                        change: [+1.2093% +1.2660% +1.3229%] (p = 0.00 < 0.05)
                        Performance has regressed.
bench_matches/gdb17_200/tree-nauty                        
                        time:   [106.56 ms 106.70 ms 106.82 ms]
                        change: [+2.0659% +2.1926% +2.3128%] (p = 0.00 < 0.05)
                        Performance has regressed.
bench_matches/checks/nauty                        
                        time:   [30.141 ms 30.181 ms 30.233 ms]
                        change: [+0.6054% +0.6953% +0.7876%] (p = 0.00 < 0.05)
                        Change within noise threshold.
bench_matches/checks/tree-nauty                        
                        time:   [13.968 ms 13.983 ms 13.995 ms]
                        change: [-0.5282% -0.4454% -0.3591%] (p = 0.00 < 0.05)
                        Change within noise threshold.
bench_matches/coconut_55/nauty                        
                        time:   [179.18 ms 179.21 ms 179.24 ms]
                        change: [+0.7371% +0.7962% +0.8556%] (p = 0.00 < 0.05)
                        Change within noise threshold.
bench_matches/coconut_55/tree-nauty                        
                        time:   [83.964 ms 84.062 ms 84.151 ms]
                        change: [+1.0854% +1.2288% +1.3658%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_bounds/gdb13_1201/no-bounds                        
                        time:   [60.920 ms 61.287 ms 61.625 ms]
                        change: [-0.8646% +0.0085% +0.8902%] (p = 0.98 > 0.05)
                        No change in performance detected.
bench_bounds/gdb13_1201/log                        
                        time:   [59.179 ms 59.745 ms 60.368 ms]
                        change: [-3.6046% -2.5497% -1.2378%] (p = 0.00 < 0.05)
                        Performance has improved.
bench_bounds/gdb13_1201/int                        
                        time:   [57.850 ms 58.321 ms 58.742 ms]
                        change: [-0.0849% +0.8798% +1.7406%] (p = 0.08 > 0.05)
                        No change in performance detected.
bench_bounds/gdb13_1201/int-vec                        
                        time:   [51.359 ms 51.791 ms 52.279 ms]
                        change: [-6.4040% -5.3993% -4.3477%] (p = 0.00 < 0.05)
                        Performance has improved.
bench_bounds/gdb13_1201/int-matchable                        
                        time:   [57.349 ms 57.749 ms 58.164 ms]
                        change: [-1.2293% -0.1072% +1.0107%] (p = 0.86 > 0.05)
                        No change in performance detected.
bench_bounds/gdb17_200/no-bounds                        
                        time:   [203.63 ms 205.38 ms 207.22 ms]
                        change: [-1.3483% -0.2853% +0.9492%] (p = 0.64 > 0.05)
                        No change in performance detected.
bench_bounds/gdb17_200/log                        
                        time:   [146.55 ms 147.24 ms 148.01 ms]
                        change: [-1.5791% -0.7497% -0.0028%] (p = 0.08 > 0.05)
                        No change in performance detected.
bench_bounds/gdb17_200/int                        
                        time:   [49.222 ms 49.394 ms 49.577 ms]
                        change: [-0.6345% +0.0591% +0.7135%] (p = 0.87 > 0.05)
                        No change in performance detected.
bench_bounds/gdb17_200/int-vec                        
                        time:   [47.898 ms 48.158 ms 48.406 ms]
                        change: [-0.8997% -0.2249% +0.4297%] (p = 0.51 > 0.05)
                        No change in performance detected.
bench_bounds/gdb17_200/int-matchable                        
                        time:   [37.749 ms 37.958 ms 38.174 ms]
                        change: [-2.4704% -1.7673% -0.9470%] (p = 0.00 < 0.05)
                        Change within noise threshold.
bench_bounds/checks/no-bounds                        
                        time:   [158.64 ms 162.03 ms 165.32 ms]
                        change: [-4.1367% -1.5621% +1.1080%] (p = 0.27 > 0.05)
                        No change in performance detected.
bench_bounds/checks/log time:   [94.643 ms 95.861 ms 97.108 ms]
                        change: [-2.7720% -1.0210% +0.7601%] (p = 0.29 > 0.05)
                        No change in performance detected.
bench_bounds/checks/int time:   [10.769 ms 10.848 ms 10.936 ms]
                        change: [-2.3658% -1.2871% -0.3041%] (p = 0.02 < 0.05)
                        Change within noise threshold.
bench_bounds/checks/int-vec                        
                        time:   [7.6748 ms 7.7125 ms 7.7489 ms]
                        change: [-1.2992% -0.3803% +0.5801%] (p = 0.44 > 0.05)
                        No change in performance detected.
bench_bounds/checks/int-matchable                        
                        time:   [6.1327 ms 6.1569 ms 6.1832 ms]
                        change: [-1.3637% -0.6708% +0.2008%] (p = 0.10 > 0.05)
                        No change in performance detected.
bench_bounds/coconut_55/no-bounds                        
                        time:   [1.7910 s 1.8408 s 1.8909 s]
                        change: [-9.6726% -5.3646% -1.3278%] (p = 0.03 < 0.05)
                        Performance has improved.
bench_bounds/coconut_55/log                        
                        time:   [1.2862 s 1.3275 s 1.3751 s]
                        change: [-10.168% -4.8163% +0.7437%] (p = 0.12 > 0.05)
                        No change in performance detected.
bench_bounds/coconut_55/int                        
                        time:   [127.90 ms 129.54 ms 131.11 ms]
                        change: [-1.2518% +0.1970% +1.8351%] (p = 0.81 > 0.05)
                        No change in performance detected.
bench_bounds/coconut_55/int-vec                        
                        time:   [113.38 ms 114.79 ms 116.30 ms]
                        change: [-3.0301% -1.6095% -0.1266%] (p = 0.05 < 0.05)
                        Change within noise threshold.
bench_bounds/coconut_55/int-matchable                        
                        time:   [42.591 ms 42.737 ms 42.887 ms]
                        change: [-1.2819% -0.8014% -0.3311%] (p = 0.00 < 0.05)
                        Change within noise threshold.

bench_memoize/gdb13_1201/no-memoize                        
                        time:   [42.556 ms 42.893 ms 43.210 ms]
                        change: [-4.0599% -3.1551% -2.1522%] (p = 0.00 < 0.05)
                        Performance has improved.
bench_memoize/gdb13_1201/nauty-index                        
                        time:   [60.254 ms 60.734 ms 61.188 ms]
                        change: [+1.4022% +2.6080% +3.7734%] (p = 0.00 < 0.05)
                        Performance has regressed.
bench_memoize/gdb13_1201/tree-nauty-index                        
                        time:   [56.153 ms 56.693 ms 57.254 ms]
                        change: [-2.4852% -1.1663% +0.2100%] (p = 0.12 > 0.05)
                        No change in performance detected.
bench_memoize/gdb17_200/no-memoize                        
                        time:   [23.467 ms 23.572 ms 23.742 ms]
                        change: [-6.2140% -1.5746% +2.9495%] (p = 0.54 > 0.05)
                        No change in performance detected.
bench_memoize/gdb17_200/nauty-index                        
                        time:   [40.806 ms 41.052 ms 41.288 ms]
                        change: [-0.8827% -0.0600% +0.7286%] (p = 0.88 > 0.05)
                        No change in performance detected.
bench_memoize/gdb17_200/tree-nauty-index                        
                        time:   [38.214 ms 38.411 ms 38.607 ms]
                        change: [-1.6246% -0.9589% -0.2082%] (p = 0.02 < 0.05)
                        Change within noise threshold.
bench_memoize/checks/no-memoize                        
                        time:   [4.1358 ms 4.1508 ms 4.1681 ms]
                        change: [-1.9856% -1.0434% -0.1242%] (p = 0.04 < 0.05)
                        Change within noise threshold.
bench_memoize/checks/nauty-index                        
                        time:   [7.0039 ms 7.0626 ms 7.1412 ms]
                        change: [-1.8450% -0.4827% +0.6898%] (p = 0.49 > 0.05)
                        No change in performance detected.
bench_memoize/checks/tree-nauty-index                        
                        time:   [6.1460 ms 6.1790 ms 6.2105 ms]
                        change: [-2.6432% -1.3694% -0.2210%] (p = 0.03 < 0.05)
                        Change within noise threshold.
bench_memoize/coconut_55/no-memoize                        
                        time:   [33.557 ms 33.704 ms 33.849 ms]
                        change: [-2.1933% -0.9271% +0.3611%] (p = 0.19 > 0.05)
                        No change in performance detected.
bench_memoize/coconut_55/nauty-index                        
                        time:   [47.577 ms 47.692 ms 47.813 ms]
                        change: [-1.2778% -0.6439% -0.0621%] (p = 0.05 < 0.05)
                        Change within noise threshold.
bench_memoize/coconut_55/tree-nauty-index                        
                        time:   [42.575 ms 42.740 ms 42.894 ms]
                        change: [-1.4038% -0.9432% -0.5088%] (p = 0.00 < 0.05)
                        Change within noise threshold.

@jdaymude jdaymude requested a review from Garrett-Pz June 15, 2026 22:24
@jdaymude jdaymude self-assigned this Jun 15, 2026
@jdaymude jdaymude added the architecture Refactors and non-functional structure improvements label Jun 15, 2026
@jdaymude jdaymude force-pushed the sure-fragmentation branch from 1065e5a to b8305c8 Compare June 15, 2026 22:28
@jdaymude jdaymude marked this pull request as ready for review June 15, 2026 22:56
@jdaymude jdaymude changed the title refactor: Remove unnecessary Option<_> from assembly::fragments Remove unnecessary Option from assembly::fragments Jun 15, 2026

@Garrett-Pz Garrett-Pz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cool, looks good.

@Garrett-Pz Garrett-Pz merged commit 4989aac into main Jun 16, 2026
11 checks passed
@jdaymude jdaymude deleted the sure-fragmentation branch June 16, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

architecture Refactors and non-functional structure improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants