Skip to content

Added 1-based indexing approach in fenwick tree #1376

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

urabhay10
Copy link

Issue reference: #1346
Solved the issues in my previous pull request and added 1-based approach as main approach throughout the article, also zero-based approach is still in a section for reference.
I tested this on some sample cases locally, feel free to test the codes again. Let me know if there are still any problems with this article. Thank you for accepting my pull request in advanced.

@jxu
Copy link
Contributor

jxu commented Oct 21, 2024

Btw you do not need to open a new PR. If you update your branch it will show in the PR.

@urabhay10
Copy link
Author

Sorry, I am new to open source and I understood my previous PR was full of mistakes so I reopened a new PR

@urabhay10
Copy link
Author

urabhay10 commented Oct 21, 2024

Also, the PR tests failed because of naming convention change it imported the wrong function to check, I've tried to fix that please re run the tests @jxu and check if this PR can be merged. Thank you

@urabhay10
Copy link
Author

@jxu Could you please review my changes and test my code if it is fit to merge

@urabhay10
Copy link
Author

@adamant-pwn Can you please review my pull request

@urabhay10
Copy link
Author

@adamant-pwn Can you please review my request?

@adamant-pwn
Copy link
Member

Hi, this is a brief non-update. Yes, I will look into this, but it might take a bit more time.

@urabhay10
Copy link
Author

No problem

@jxu
Copy link
Contributor

jxu commented Nov 4, 2024

Off-topic: we should mention the Fenwick tree was actually first proposed by Boris Ryabko in 1989. It was published in Russian in Soviet Math. Doklady which is probably why it wasn't picked up.

@jxu jxu self-requested a review November 4, 2024 18:18
**Note:** The Fenwick tree presented here uses zero-based indexing.
Many people use a version of the Fenwick tree that uses one-based indexing.
As such, you will also find an alternative implementation which uses one-based indexing in the implementation section.
**Note:** The Fenwick tree presented here uses one-based indexing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Emphasize this is for didactic reasons, i.e. to make the indexing and ranges easier.

Many people use a version of the Fenwick tree that uses one-based indexing.
As such, you will also find an alternative implementation which uses one-based indexing in the implementation section.
**Note:** The Fenwick tree presented here uses one-based indexing.
Many people use a version of the Fenwick tree that uses zero-based indexing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Most presentations I've seen are with one-based indexing.

1. First, it adds the sum of the range $[g(r), r]$ (i.e. $T[r]$) to the `result`.
2. Then, it "jumps" to the range $[g(g(r)-1), g(r)-1]$ and adds this range's sum to the `result`.
3. This continues until it "jumps" from $[0, g(g( \dots g(r)-1 \dots -1)-1)]$ to $[g(-1), -1]$; this is where the `sum` function stops jumping.
1. First, it adds the sum of the range $(g(r), r]$ (i.e. $T[r]$) to the `result`.
Copy link
Contributor

Choose a reason for hiding this comment

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

result technically should be res

2. Then, it "jumps" to the range $[g(g(r)-1), g(r)-1]$ and adds this range's sum to the `result`.
3. This continues until it "jumps" from $[0, g(g( \dots g(r)-1 \dots -1)-1)]$ to $[g(-1), -1]$; this is where the `sum` function stops jumping.
1. First, it adds the sum of the range $(g(r), r]$ (i.e. $T[r]$) to the `result`.
2. Then, it "jumps" to the range $(g(g(r)), g(r)]$ and adds this range's sum to the `result`.
Copy link
Contributor

Choose a reason for hiding this comment

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

By applying g to the start and end indices

1. First, it adds the sum of the range $[g(r), r]$ (i.e. $T[r]$) to the `result`.
2. Then, it "jumps" to the range $[g(g(r)-1), g(r)-1]$ and adds this range's sum to the `result`.
3. This continues until it "jumps" from $[0, g(g( \dots g(r)-1 \dots -1)-1)]$ to $[g(-1), -1]$; this is where the `sum` function stops jumping.
1. First, it adds the sum of the range $(g(r), r]$ (i.e. $T[r]$) to the `result`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The r here is the initial value of r. In the code it changes per loop.


<center>![Binary Indexed Tree](binary_indexed_tree.png)</center>
As you can see, the main benefit of this approach is that the binary operations complement each other very nicely.

## Implementation

### Finding sum in one-dimensional array

Here we present an implementation of the Fenwick tree for sum queries and single updates.
Copy link
Contributor

Choose a reason for hiding this comment

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

You must state the API for the tree is 0-based as usual, even though internally it is 1-based. It is confusing

@@ -181,22 +153,22 @@ struct FenwickTree {
The above implementation requires $O(N \log N)$ time.
It's possible to improve that to $O(N)$ time.

The idea is, that the number $a[i]$ at index $i$ will contribute to the range stored in $bit[i]$, and to all ranges that the index $i | (i + 1)$ contributes to.
The idea is, that the number $a[i]$ at index $i$ will contribute to the range stored in $bit[i+1]$, and to all ranges that the index $i + (i ~\&~ (-i))$ contributes to.
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary comma after "is"

@@ -208,23 +180,23 @@ struct FenwickTreeMin {

FenwickTreeMin(int n) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this code present then? Should we say segment tree can solve this problem for min ranges as min is associative?


For this approach we change the requirements and definition for $T[]$ and $g()$ a little bit.
We want $T[i]$ to store the sum of $[g(i)+1; i]$.
We want $T[i]$ to store the sum of $[g(i); i]$.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to store [g(i), i) instead? This may be a separate PR.

@@ -68,7 +68,7 @@ void test_fenwick_sum_onebased() {
using namespace Fenwick_Sum_Onebased;
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming should be consistent. We have FenwickTree 1-based as the default, so the other one should be called 0-based.


$$h(j) = j ~|~ (j+1),$$
$$g(i) = i - (i ~\&~ (-i)).$$
Copy link
Contributor

@jxu jxu Nov 8, 2024

Choose a reason for hiding this comment

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

This can also be implemented as r & (r-1), and that's what gcc will optimize it to.
Gcc didn't have any trick for the inverse operation h.

@jxu jxu mentioned this pull request Jan 6, 2025
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.

3 participants