Skip to content

Partial 325 Fix P2 - Updated vectvshash.png Added - #1138

Open
GalinaP7 wants to merge 4 commits into
pearcej:masterfrom
GalinaP7:2.8Listing6p2
Open

Partial 325 Fix P2 - Updated vectvshash.png Added#1138
GalinaP7 wants to merge 4 commits into
pearcej:masterfrom
GalinaP7:2.8Listing6p2

Conversation

@GalinaP7

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1137 - adds the replacement graph for fig-vectvshash-cpp, which was commented out in that PR since the old
image reflected the previous (incorrect) cumulative-timing measurement.

fixes #325

What's fixed

image

New chart generated from corrected code output

  • Ran the corrected C++ example (std::chrono-based, per-operation average timing) locally across the full range (10,000 to 990,000 elements, step 20,000).
  • Built a new vectvshash.png in Excel from that real output, replacing the old image which was based on cumulative search time (0–50 second scale) rather than per-operation average time (microsecond/nanosecond scale).
  • Converted vector search time to microseconds for readability (raw values were in the 1e-07 to 1e-05 second range, which rendered as hard-to-read scientific notation on a linear axis).
  • Axis titles, legend labels (Vector / Hash Table), and marker shapes (circle for vector, triangle for hash table) were set to match the style of the original figure.

Figure re-enabled

  • Uncommented the <figure xml:id="fig-vectvshash-cpp"> block in HashTableAnalysis.ptx.
  • Restored the <xref ref="fig-vectvshash-cpp"/> in the results paragraph that references it.
  • Updated the figure's <description> text to describe the actual new image: vector climbs to approximately 69 microseconds at 1,000,000 elements, hash table stays flat at approximately 2–3 nanoseconds throughout (previously said "45 seconds," which
    reflected the old, incorrect measurement).

Tested on local build

Reviewed by @harrisonj2-v

@GalinaP7 GalinaP7 changed the title Updated vectvshash.png Added Partial P2 - Updated vectvshash.png Added Aug 13, 2026
@GalinaP7 GalinaP7 changed the title Partial P2 - Updated vectvshash.png Added Partial 325 Fix P2 - Updated vectvshash.png Added Aug 13, 2026
@wrigjl

wrigjl commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Am I missing something (quite possible)? I see that you've updated the text, but I don't see the new image in the PR (missing a "git add"?).

@GalinaP7

Copy link
Copy Markdown
Contributor Author

That's completely my fault! I forgot to attach the PNG.

@wrigjl wrigjl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Heh, happens to everyone =)

@pearcej pearcej left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is a big improvement! However, I have a minor nit. I would rather the xml:id be fig-vectvshash2-cpp rather than fig-vectvshash-cpp2 because I think semantically the change is not to the language, but the figure.

@pearcej

pearcej commented Aug 15, 2026

Copy link
Copy Markdown
Owner

@wrigjl Thoughts on this and part 1 (#1137)?

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.

Listing 6 in Section 2.8 does not test the contains operation

3 participants