deprecate uniform random winnowing - #19
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR deprecates and removes the uniform_random winnowing thresholding mode across the implementation, default configuration, and documentation, and adds/updates README guidance for syncing data directories between environments.
Changes:
- Removed the
uniform_randomthresholding branch fromrun_thresholding. - Updated the shipped
winnowing.yamland README example to removeuniform_randomand showduration_s. - Added additional README sync instructions (symlinks +
rsync) for moving the repo and data-directory layout between systems.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/cocoa/winnower.py |
Removes uniform_random thresholding code path and leaves duration/first-occurrence options. |
src/cocoa/config/winnowing.yaml |
Removes uniform_random from the default config and enables duration_s in the template. |
README.md |
Removes uniform_random from docs/examples and adds additional environment sync instructions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| ) | ||
| else: | ||
| raise NotImplementedError("Please check the thresholding configuration.") |
Comment on lines
11
to
13
| # choose one and only one of the following | ||
| # duration_s: !!int 86400 # 24h | ||
| duration_s: !!int 86400 # 24h | ||
| # first_occurrence: XFR-IN//icu |
Comment on lines
446
to
450
| threshold: | ||
| # choose one and only one of the following | ||
| # duration_s: !!int 86400 # 24h | ||
| duration_s: !!int 86400 # 24h | ||
| # first_occurrence: XFR-IN//icu | ||
| uniform_random: !!bool True | ||
|
|
Comment on lines
+627
to
647
|
|
||
| Send to randi: | ||
| ``` | ||
| for d in data-raw processed; do | ||
| ln -s /gpfs/data/bbj-lab/users/burkh4rt/$d $d | ||
| done | ||
| ``` | ||
| ``` | ||
| rsync -avh \ | ||
| --exclude "output" \ | ||
| --exclude "processed" \ | ||
| --exclude "data-raw" \ | ||
| --exclude "logs" \ | ||
| --exclude "wandb" \ | ||
| --exclude ".venv/" \ | ||
| --exclude ".idea/" \ | ||
| ~/Documents/chicago/cocoa \ | ||
| randi:/gpfs/data/bbj-lab/users/burkh4rt | ||
| ``` | ||
|
|
||
| --> |
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 pull request removes support for the
uniform_randomthresholding option throughout the codebase and documentation, and adds improved instructions for syncing data directories. The most important changes are summarized below.Thresholding logic and configuration:
uniform_randomthresholding option from both the YAML configuration (src/cocoa/config/winnowing.yaml) and the example in the documentation (README.md). [1] [2]uniform_randomthresholding in therun_thresholdingmethod inwinnower.py, so this option is no longer available in code.Documentation and usability:
rsyncto sync data directories between local and remote environments in theREADME.md.