[FIX] Forward the extraction thread count to the integration-test notebook - #558
Draft
noel-improv wants to merge 1 commit into
Draft
noel-improv wants to merge 1 commit into
noel-improv wants to merge 1 commit into
Conversation
…st notebook build-tests.sh writes EXTRACTION_NUM_WORKERS and EXTRACTION_BATCH_SIZE into the notebook's environment file, but not EXTRACTION_NUM_THREADS_PER_WORKER, so a run always extracted at the default of four threads whatever was set locally. A thread or worker sweep could not vary the setting it was measuring.
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.
Description
build-tests.shwrites the extraction settings a run needs into the notebook's.env.testing. #490 addedEXTRACTION_NUM_WORKERSandEXTRACTION_BATCH_SIZEthere, but notEXTRACTION_NUM_THREADS_PER_WORKER, so every run extracted at the default of four threads regardless of what was set locally. This forwards it the same way as its siblings and documents it inenv.template.Changes
integration-tests/build-tests.sh: writeEXTRACTION_NUM_THREADS_PER_WORKERto.env.testingwhen it is set, using the sameprintf %qform as the settings next to it.integration-tests/env.template: addEXTRACTION_NUM_THREADS_PER_WORKERwith its default of 4.Problem
A thread or worker sweep cannot vary the setting it is measuring. It showed up on the single-worker drain re-run for #493, which needed 32 threads and would otherwise have run the 4-thread arm.
Related issue (if any): #493
Testing
pytest)bash -npasses. Running the new block on its own writesexport EXTRACTION_NUM_THREADS_PER_WORKER=32when the variable is set and writes nothing when it is unset. The same line carried a real run: a stack launched withEXTRACTION_NUM_THREADS_PER_WORKER=32, and the notebook's shipped.env.testingcontained that value.Checklist
No new files. Unset, the script behaves as before.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.