fix(algo): apply tg_wcc print_limit to the component maps - #186
Open
eastagiletracker wants to merge 1 commit into
Open
Conversation
print_limit bounded only the printed vertex set. @@comp_sizes_map and @@comp_group_by_size_map hold one entry per connected component, so on a graph with many components the response kept growing regardless of the configured limit and could exceed the result size limit. Truncate both maps to print_limit as well. print_limit = -1 still prints everything, so existing callers see identical output. Adds regression tests over the Empty graph, where every vertex is its own component.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes applying
tg_wcc'sprint_limitto the per-component maps it prints, so a run on a graph with many components stays within the result size limit (Fixes #127). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/279. You can sign in with your GitHub ID to claim ownership of the project.What is wrong today
print_limitbounds only the printed vertex set.@@comp_sizes_mapholds one entry per connected component and@@comp_group_by_size_mapholds every component id grouped by size, and both are printed in full no matter whatprint_limitis set to. On a large sparse graph the component count is the thing that grows, so the part of the responseprint_limitwas meant to cap is exactly the part that stays uncapped.Reproduction at current HEAD
On
tg_4.4.0_dev(c8ca472), with a stock engine and 20 isolatedV20vertices — so 20 single-vertex components —tg_wccwas installed unmodified and run withprint_limit=5:The vertex list honors the limit and the two maps ignore it:
On that same 20-component run the two maps are most of the payload, and on a 50-component version of it they were 1228 of the 1719 response bytes at
print_limit=5against 378 bytes for the capped vertex list. The maps grow with the component count while the capped part does not.The change
The
FOREACHthat builds the grouped map now stops afterprint_limitcomponents and fills a@@print_comp_sizes_mapalongside it, which is what gets printed assizes.print_limit < 0keeps the documented "print everything" path, so existing callers are unaffected. The parameter comments were updated to say the limit covers components as well as vertices. Nothing in the traversal, theresult_attributewrite path or thefile_pathCSV path is touched.Verification
Same engine, same graphs, before and after. With
print_limit=-1the full response is byte-identical to the pre-change run (10847 bytes on the 60-component graph), and every vertex's component assignment is unchanged at every limit — the capped maps are strict subsets of the uncapped ones, so no value is altered, only omitted. Edge values behave:print_limitof 0, 1, 59, 60 and 1000 against 60 components return 0, 1, 59, 60 and 60 entries, andprint_results=falsestill prints nothing.tests/test/test_community.pygainstest_wcc_print_limitover theEmptygraph, where every vertex is its own component. Against the unmodified query the three limits fail and the-1case passes:With the change applied,
4 passed. Running the whole oftest_community.pybefore and after leaves the same 15test_lcc*failures either way — those look fordata/baseline/graph_algorithms_baselines/..., which is not in the repo — so this adds no new failures.How this was managed
This work was tracked as tg_wcc query parameter output_limit not effective on a board at https://eastagiletracker.com/projects/279 that was imported from this repository's own issues and pull requests — 185 of them — and used to manage the change.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com