Skip to content

Conversation

upsj
Copy link
Member

@upsj upsj commented Jan 11, 2025

Extracted from #1758, this only contains the MST preprocessing step.

Related literature: A. Fallin, A. Gonzalez, J. Seo, and M. Burtscher, “A High-Performance MST Implementation for GPUs,” in Proceedings of the International Conference for High Performance Computing, Networking, Storage and Analysis, in SC ’23. New York, NY, USA: Association for Computing Machinery, Nov. 2023, pp. 1–13. doi: 10.1145/3581784.3607093.

@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:41
@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. reg:benchmarking This is related to benchmarking. type:factorization This is related to the Factorizations reg:helper-scripts This issue/PR is related to the helper scripts mainly concerned with development of Ginkgo. mod:all This touches all Ginkgo modules. labels Jan 11, 2025
@MarcelKoch MarcelKoch self-requested a review January 13, 2025 13:18
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.

Mostly LGTM. I have some questions about the distinction between the device and non-device code paths.
Also I think it would be helpful if you add the literature reference to the code.

Copy link
Member

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

first part of my review

@upsj upsj force-pushed the cholesky-mst-extracted branch from fce1842 to 8656cdf Compare January 15, 2025 09:57
@upsj upsj requested review from MarcelKoch and yhmtsai January 15, 2025 09:58
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 still prefer changing the comment about the edge weights, but it's not a blocker.

@upsj upsj added the 1:ST:no-changelog-entry Skip the wiki check for changelog update label Jan 15, 2025
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 52.23214% with 107 lines in your changes missing coverage. Please review.

Project coverage is 89.35%. Comparing base (75b134a) to head (706d584).

Files with missing lines Patch % Lines
test/factorization/cholesky_kernels.cpp 0.00% 55 Missing ⚠️
core/factorization/symbolic.cpp 7.14% 26 Missing ⚠️
omp/factorization/elimination_forest_kernels.cpp 40.00% 24 Missing ⚠️
core/device_hooks/common_kernels.inc.cpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1765      +/-   ##
===========================================
- Coverage    89.73%   89.35%   -0.38%     
===========================================
  Files          795      797       +2     
  Lines        65800    65965     +165     
===========================================
- Hits         59045    58946      -99     
- Misses        6755     7019     +264     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

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

second part of my review. some question might be dumb after I go through the algorithm.
Will do the algorithm part later

@upsj upsj requested a review from yhmtsai January 16, 2025 10:31
@upsj upsj added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review labels Jan 16, 2025
@upsj upsj merged commit 0b3436e into develop Jan 16, 2025
10 of 11 checks passed
@upsj upsj deleted the cholesky-mst-extracted branch January 16, 2025 13:53
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. 1:ST:run-full-test mod:all This touches all Ginkgo modules. reg:benchmarking This is related to benchmarking. reg:build This is related to the build system. reg:helper-scripts This issue/PR is related to the helper scripts mainly concerned with development of Ginkgo. reg:testing This is related to testing. type:factorization This is related to the Factorizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants