Fix maximum Fenwick tree range values after updates

[dhairyajangir]

Describe your change

• Add an algorithm?
• Fix a bug or typo in an existing algorithm?
• Add or change doctests? -- Added at the maintainer's explicit request; all existing tests are preserved.
• Documentation change?

MaxFenwickTree.update() currently stores max(value, current_left_border, index) in internal nodes. Indices become candidate values, and previous maxima are lost. For example:

tree = MaxFenwickTree(8)
tree.update(4, 20)
tree.update(5, 1)
tree.query(0, 6)  # currently 5; should be 20

Performance feedback from the maintainer is addressed by c25e1e9e3f81407e3f2205a842b0f2907aad8b94.

The revised updater preserves arbitrary overwrites and decreases while limiting recomputation:

• Equal assignments return immediately.
• A new value exceeding a bucket's maximum updates that maximum directly.
• Lowering the previous maximum recomputes from the node's own value and its disjoint child Fenwick buckets.
• An unchanged maximum stops propagation to ancestors; recovering a tied previous maximum also stops the child scan.

Query code and the existing zero-floor semantics for negative-only ranges are unchanged. The worst-case update bound remains O(log² N).

The maintainer requested regressions and a 2,000-item benchmark. All 21 original examples remain verbatim, with 12 previously added examples retained (33 total); the original updater fails four new result assertions. This performance revision changes only update(), preserving all 33 examples and the complete benchmark block.

Fresh best-of-five timings on CPython 3.15.0rc2 free-threaded, identical workload and setup outside timing:

Workload	Original buggy updater	Previous correct fix	Revised
Existing 4,000 mixed updates	0.007629 s	0.121388 s	0.004207 s
4,000 decreasing updates	0.008302 s	0.122074 s	0.037279 s
4,000 updates repeatedly removing the unique maximum	0.015192 s	0.221192 s	0.034707 s
2,000 queries from identical correct buckets	0.012316 s	0.014007 s	0.013374 s

The mixed workload is faster in this run, but decreases and repeated unique-maximum removal remain 4.49× and 2.285× slower than the broken original. This is not a universal speedup or performance-parity claim. The unchanged query timings reflect noise. The original mixed update workload leaves 428 wrong suffix answers; both correct versions give zero. Additional increasing, tied-maximum and unchanged-value workloads also pass, with full-range answers checked after every update and final suffix answers/buckets verified.

Validation: all 33 doctests, the targeted pytest module doctest, and every applicable configured pre-commit hook passed under CPython 3.15.0rc2 free-threaded. External regression checks passed 1,053,076 range comparisons and 46,830 exact bucket comparisons across arbitrary overwrites, ties, decreases, signed values and non-power-of-two sizes. Independent review separately verified the bucket invariants and additional signed sequences. AI assistance was used for implementation and independent review.

Checklist

• I have read CONTRIBUTING.md.
• This pull request is all my own work -- I have not plagiarized. (AI assistance disclosed above; this personal attestation is left for the contributor.)
• I know that pull requests will not be merged if they fail the automated tests.
• This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
• All new Python files are placed inside an existing directory. (Not applicable: no new files.)
• All filenames are in all lowercase characters with no spaces or dashes.
• All functions and variable names follow Python naming conventions.
• All function parameters and return values are annotated with Python type hints.
• All functions have doctests that pass the automated testing. (All 33 class examples pass; existing method docstrings remain unchanged.)
• All new algorithms include at least one URL that points to Wikipedia or another similar explanation. (Not applicable: no new algorithm.)
• If this pull request resolves one or more open issues, then the description above includes the issue number(s) with a closing keyword. (No linked issue.)

[cclauss]

Please add one or more tests (without removing or modifying any existing tests) that fail with the current algorithm but pass with the proposed algorithm.

Please add a timeit or similar benchmark that measures the performance difference on a large tree (like 2k items).

[dhairyajangir]

Implemented in d49aade77d8531ab39e15b0df111b95a0a6585b5.

Added 12 doctest examples covering preserved sibling maxima, decreasing/resetting the maximum, and the ancestor's own value. All 21 existing examples are unchanged. The original algorithm fails the four new result assertions; the proposed version passes all 33 examples.

The same file now has an optional benchmark:

python3.15t data_structures/binary_tree/maximum_fenwick_tree.py --benchmark

Using the identical benchmark block with the original algorithm from 84b73d08 and the proposed algorithm, on CPython 3.15.0rc2 free-threaded (GIL disabled), pinned to one CPU, best of five repetitions:

Workload on 2,000 items	Original	Proposed
4,000 updates (populate, then overwrite including decreases)	0.017630 s	0.113037 s
2,000 queries on identical correct buckets	0.013313 s	0.013777 s

Updates are about 6.41× slower because they now recompute correct range maxima. The original update workload leaves 428 of 2,000 suffix-query results incorrect; the proposed version leaves zero. Query code is unchanged; the small observed timing difference does not establish a query-performance change. Setup and correctness checks are outside timing; every update repetition starts with a fresh zero tree, and query repetitions use identical correct buckets prepared outside timing.

All applicable pre-commit run --all-files --show-diff-on-failure hooks pass. Independent review verified the retained tests, regressions, benchmark setup and correctness results. No production methods were changed in this follow-up.

Reproduce the original-algorithm benchmark with the identical harness

From the repository root, export the original class with the new benchmark block:

from pathlib import Path
from subprocess import check_output

path = "data_structures/binary_tree/maximum_fenwick_tree.py"
base = "84b73d08f8bfa4e6bfae0243369c24f5a7539745"
original = check_output(["git", "show", f"{base}:{path}"], text=True)
proposed = Path(path).read_text()
marker = 'if __name__ == "__main__":'
Path("/tmp/maximum_fenwick_tree_before.py").write_text(
    original[:original.index(marker)] + proposed[proposed.index(marker):]
)

taskset -c 0 nice -n 10 python3.15t /tmp/maximum_fenwick_tree_before.py --benchmark
taskset -c 0 nice -n 10 python3.15t data_structures/binary_tree/maximum_fenwick_tree.py --benchmark

[cclauss]

These multiples are too high. We will keep the current implementation until we find a more performant solution.

[dhairyajangir]

Published in c25e1e9e3f81407e3f2205a842b0f2907aad8b94.

I revised the updater to avoid recomputing maxima that cannot change. Increasing a bucket maximum now assigns it directly; decreasing a previous maximum combines disjoint child Fenwick buckets. Propagation stops once a bucket maximum is unchanged. Arbitrary decreases, ties, and the existing negative-value behavior are preserved.

The same 2,000-item benchmark now gives these fresh best-of-five times on CPython 3.15.0rc2 free-threaded. Each update workload contains 4,000 assignments; setup and correctness checks are outside timing.

Workload	Original updater	Previous correct fix	Revised
Existing mixed updates	0.007629 s	0.121388 s	0.004207 s
Increasing updates	0.007937 s	0.118012 s	0.004311 s
Decreasing updates	0.008302 s	0.122074 s	0.037279 s
Tied maxima	0.007313 s	0.115037 s	0.004060 s
Repeated unique-maximum removal	0.015192 s	0.221192 s	0.034707 s
Unchanged assignments	0.009548 s	0.119459 s	0.000355 s
2,000 queries from identical correct buckets	0.012316 s	0.014007 s	0.013374 s

The worst cases still cost more than the original: decreasing updates are 4.49×, and repeated unique-maximum removal 2.285×, slower than the broken updater. The worst-case bound remains O(log² N). Query code is unchanged; its small timing differences are noise. I am not claiming universal speed parity.

Both correct versions have zero incorrect full-range answers, final suffix answers, or final buckets in these workloads. The original mixed workload leaves 428 incorrect suffix answers. The revision also passes all 33 unchanged doctests, the module's pytest doctest, 1,053,076 external range comparisons, 46,830 exact bucket comparisons, and every applicable configured pre-commit hook. Independent review separately checked signed overwrites and tied maxima.

The existing --benchmark code is unchanged, so the mixed-workload comparison remains reproducible with the same main block on each revision. The two adversarial workloads use:

• Decreases: initialize 2,000 values with 4000 - index; assign 2000 - index, then -index, in increasing index order.
• Unique-max removal: start with zeros and alternate update(0, 1) / update(0, 0) 2,000 times.

For those comparisons, each repetition starts from fresh, correct buckets constructed from the initial array outside timing. These remaining costs are included explicitly so you can assess whether this narrower repair meets the performance requirement. AI assistance was used for implementation and independent review.