Skip to content

Conversation

upsj
Copy link
Member

@upsj upsj commented Jan 11, 2025

This adds a data structure to answer queries of the kind $\text{argmin}_{i \in [a,b]} \text{array}[i]$ in constant time. This will be used to speed up the computation of LCAs in the symbolic Cholesky algorithm, turning the counting step from $O(nnz(L))$ algorithm to a $O(nnz(A))$ algorithm.

The corresponding Wikipedia articles describe the algorithms pretty well https://en.wikipedia.org/wiki/Range_minimum_query, the constant time query is described in more detail in J. Fischer and V. Heun, “Space-Efficient Preprocessing Schemes for Range Minimum Queries on Static Arrays,” SIAM J. Comput., vol. 40, no. 2, pp. 465–492, Jan. 2011, doi: 10.1137/090779759.

TODO:

  • add test for device queries

@upsj upsj added the 1:ST:ready-for-review This PR is ready for review label Jan 11, 2025
@upsj upsj requested a review from a team January 11, 2025 13:33
@upsj upsj self-assigned this Jan 11, 2025
@ginkgo-bot ginkgo-bot added reg:build This is related to the build system. reg:testing This is related to testing. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:reference This is related to the reference module. mod:hip This is related to the HIP module. labels Jan 11, 2025
@yhmtsai
Copy link
Member

yhmtsai commented Jan 13, 2025

my first impression on turning $O(nnz(L))$ to $O(nnz(A))$ is more operation.
do I miss something?

@upsj
Copy link
Member Author

upsj commented Jan 13, 2025

L includes fill-in, A does not

@MarcelKoch MarcelKoch self-requested a review January 14, 2025 08:32
Copy link
Member

@MarcelKoch MarcelKoch left a comment

Choose a reason for hiding this comment

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

Overall, a lot more documentation is necessary. Especially what the new classes in bit_packed_storage and range_minumum_querry are there for, and how they work together.
At least I can't properly review it right now, since there is too little information on what functions/classes are supposed to do.

Co-authored-by: Marcel Koch <marcel.koch@kit.edu>
@upsj upsj requested review from a team and MarcelKoch January 19, 2025 21:44
Copy link
Member

@MarcelKoch MarcelKoch left a comment

Choose a reason for hiding this comment

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

I would like to have the discussion on the description of lookup kernels resolved. The rest are smaller nits.

}
}

/** Returns the ballot number C(p, q). */
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to describe in a few words the meaning of C(p, q) in this context?

Copy link
Member Author

@upsj upsj Jan 20, 2025

Choose a reason for hiding this comment

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

I don't think there is any particularly useful description I could add here, beyond the definition above.

@upsj upsj added 1:ST:ready-to-merge This PR is ready to merge. 1:ST:no-changelog-entry Skip the wiki check for changelog update and removed 1:ST:ready-for-review This PR is ready for review 1:ST:run-full-test labels Jan 20, 2025
@upsj upsj merged commit efbf245 into develop Jan 20, 2025
10 of 11 checks passed
@upsj upsj deleted the rmq branch January 20, 2025 21:54
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-to-merge This PR is ready to merge. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:hip This is related to the HIP module. mod:reference This is related to the reference module. reg:build This is related to the build system. reg:testing This is related to testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants