Skip to content
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

Fix bit order of RMAT Rectangular Generator to match expectation #2542

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

mfoerste4
Copy link
Collaborator

Previously, the decimal indices of rows/columns of the adjacency matrix did not align with the node-ids created by the algorithm. This PR fixes the bits set for each decision during the computation as described by the docstring.

FYI @tfeher

@mfoerste4 mfoerste4 requested a review from a team as a code owner January 14, 2025 22:20
@github-actions github-actions bot added the cpp label Jan 14, 2025
@mfoerste4 mfoerste4 added bug Something isn't working and removed cpp labels Jan 14, 2025
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Malte, this looks good! Just one additional request: it would be great to update the docstring of thetha to be explicit about which element of the array describes which bit level.

I would recommend to start with the main docstring, and replace

* This is the most general of several overloads of `rmat_rectangular_gen`
* in this file, and thus has the most detailed documentation.

with

* This function generates a random graph represented by a (sparse) adjacency matrix. As described
* in [1], to generate connections, we recursively subdivide the adjacency matrix into four
* equal-sized partitions, and distribute edges within these partitions with a unequal
* probabilities. The probabilities are described by numbers [a, b, c, d]. We chose the upper left
* partition with probability `a`. The chosen partition is again subdivided into four smaller
* partitions, and the procedure is repeated until we reach a single element (1 × 1 partition).
*
* We can prescribe different probability distribution at each iteariton. The `theta` array stores
* the probability values for each level.
*
* [1] "R-MAT: A Recursive Model for Graph Mining" Deepayan Chakrabarti, Yiping Zhan, Christos
* Faloutsos (2004) https://doi.org/10.1137/1.9781611972740.43

After this, we can clarify the layout of theta by replacing

* @param[in] theta distribution of each quadrant at each level of resolution.
* Since these are probabilities, each of the 2x2 matrices for
* each level of the RMAT must sum to one. [on device]
* [dim = max(r_scale, c_scale) x 2 x 2]. Of course, it is assumed
* that each of the group of 2 x 2 numbers all sum up to 1.

with

* @param[in]  theta   array [on device] with the distribution of each quadrant at each level of
*                     resolution. theta = [a0, b0, c0, d0, a1, b1, c1, d1, ...], where 
*                     [a0, b0, c0, d0]  defines the probability at the finest level (2x2).
*                     The last four elements in the array describe the probability in the
*                     coarsest level (where matrix size = [2^r_scale, 2^c_scale]).
*                     Since these are probabilities, the four [a_i, b_i, c_i, d_i] values for
*                     each level of the RMAT must sum to one.
*                     [dim = max(r_scale, c_scale) x 2 x 2]. 

What do you think?

@github-actions github-actions bot added the cpp label Jan 22, 2025
@cjnolet cjnolet added the non-breaking Non-breaking change label Jan 22, 2025
@mfoerste4
Copy link
Collaborator Author

@tfeher , thanks for the good doc-string suggestion, I applied it to the branch.

@mfoerste4 mfoerste4 requested a review from tfeher January 24, 2025 18:01
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Malte for the update, LGTM!

@tfeher
Copy link
Contributor

tfeher commented Jan 24, 2025

/merge

@rapids-bot rapids-bot bot merged commit fc4c490 into rapidsai:branch-25.02 Jan 24, 2025
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

3 participants