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

Update open once read many test #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

briangow
Copy link
Collaborator

@tcpan , as discussed I've updated the open once, read many test. This PR makes the following changes:

  • Removes the random channel test (this was deemed unnecessary since it doesn't make sense to compare the result of this test across formats given the random nature)
  • Simplifies the logic for selecting channels, now that the random test is not being run
  • Prints the sum of the open, read, close measurements as "total"
  • Makes printing of the open, read, close measurements themselves optional (only happens when verbose is set)
  • Updates the output to the summary file (to capture total cpu time)

Committer: Brian Gow <briangow@mit.edu>
@briangow briangow requested a review from tcpan December 23, 2024 20:03
@tcpan
Copy link
Contributor

tcpan commented Jan 2, 2025

I looked through this and the logic to summarize the time looks good.

Re. random channel:
The original purpose was to avoid caching skewing the measurements. However, as discussed with Brian, Benjamin, and Eric, this will allow different channel types to be mixed in the measurement. Two alternatives were discussed that do not involve random channel selection: 1. randomize the time window to read, and 2. read the same time window but exclude the first iteration of the run when computing median.

I will do some testing on this.

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.

2 participants