Skip to content

Refactor - #44

Merged
jtb324 merged 34 commits into
mainfrom
refactor
Apr 30, 2026
Merged

Refactor#44
jtb324 merged 34 commits into
mainfrom
refactor

Conversation

@jtb324

@jtb324 jtb324 commented Apr 30, 2026

Copy link
Copy Markdown

No description provided.

jtb324 and others added 30 commits November 24, 2025 09:37
…used while refactor some of the code

The IBD filtering needs to be refactored in DRIVE but there is a delicate balance between memory and CPU time. We need to be able to track whether we are actually regressing (either or CPU time or memory) and this new folder contains scripts to test the changes in the code
…ort complexity

There were several models within the network class and utilities that were used outside of the module and no longer made sense to house them within the module. Also utilities has now become a subcommand of DRIVE some it make sense to remove the inner modules.
Created a benchmark for the polars filter. The benchmark test is parametrized with the appropriate inputs and uses 3 test data sources. One for a small input (1,000 samples), one for a medium input (70k), and a large input (250k sample). This files are all inserted at runtime of the tests.
…t files in the code

After refactor the src structure, I forgot to include the path to the phecode_mappings *.txt files in the "git add". This is corrected now so that the files are under version control
… filtering function

We need to see how the filtering function is performing in pandas compared to the polars function. I pulled out most parts (There are still some nuances) to repeat the filtering and then inserted it into the same kind of benchmark test using the same inputs as the polars test
…heir own module

Mulitple parts of the code base now need to filter thee IBD segments to just those regions of interest. I created a filters module that will have scripts used to handle this file filtering. There are filters version for pandas, polars (using parquet), and duckdb because we are moving to support those as inputs for performance benefits
Added duckdb as a dependency since we are experimenting with using it to read in large files
The indices code is very verbose with the OOP heavy classes. I created an new version that relies on a namedtuple type IbdFileIndices as the type for the indices object. Depending on what ibd_file_format option the user provides, the code now creates the object with the proper indices. This function is not used in the wider codebase yet but will soon replace the other way
…/filter.py

Goal is to move the filter in networks/filters/filter.py to be outside of the module so that other parts of the code base are not reaching into the network module to interact with the ibd file. This first step begins that decoupling process and fixes broken unit test.
Created a class that can properly generate the DuckDB SQL strings that will be used to filter the IBD files before going into the clustering analysis. Added a function to the ibd_file_reader.py script that uses duckdb to read int all of the IBD segments while filtering for who is in the cohort, what is the minimum segment size in the analysis, and whether the size overlapped or contained the locus of interest. Also finalized the filters in the loci_filter.py script
…es to represent the IDB file indices

To reduce code complexity, i switched the IBD indices object from heavy OOP structure to just namedtuples. This makes the syntax easier to read and reduces the complexity.
… listed as a dictionary in the script. I updated the annotations in the return method to be a list[str] and then made sure that the cohort variable wa sinitialized to an empty list in the negative branch of the conditional.
added an accidental double-quote to the f-string when it should've been a single quote
Moved duckdb and polars from dev dependencies to normal dependencies because they are being fully incorporated into the codebase
…o the network algorithm. The network now initializes the DuckDB filter, creates the SQL template string, and then uses the SQL to read and filter the IBD file. We then use these filtered IBD segements to generate a pandas dataframe to represent all edges in the cohort and another pandas dataframe to represent all of the vertices in the cohort.
This method was not labeled as a @staticmethod despite the fact that it doesn't need the self parameter. Now it is
…use new duckdb filtering steps

Adjust the different functions within the dendrogram subcommand to use the duckdb filter to read in the file and to use polars to filter data
…ng in ibd files method

Connected all the parts of the new duckdb filter and then fixed the bugs that were breaking the unit testing
…ed as expected

Scripts test the following four conditions: 1) make a query to filter for samples using the overlaps loci filter 2) make a query to filter for samples using the contains filter 3) make a query without filtering for samples using the overlaps filter 4) make a query to filter without filtering for samples using the contains filter. All test use a fixture with the simultated chr20 IBD data and all test check against a expected SQL statement
…e duckdb filter

Since the duckdb filter is integrated, I removed the sandbox area that was being used as a playground to intialially design how the new code would work
…rformance of the file reader script

added a benchmark that profiles the functions involved in reading in and preparing the IBD files to be read into a graph. This benchmark exceeds just a single function because it is import how that whole section of code is functioning as a whole. It test samples at 1k, 70k, and 250k participants
DuckDB can throw an InvalidInputError if the input IBD file doesn't exists. I added a check in the network_algorithm.py to make sure this file exists and then a try catch block in the ibd_file_reader.py incase the conn.execute throws the exception
A stale graph was being passed to the redo_clustering function and behavior would change based on the order that individuals were being read in. Additionally the wrong attribute was being used to read the haplotypes from the networks. This attribute was being used as a hack for buggy behavior in igraph about what metadata the vertices have. That behavior now seems to be behavior appropriately so we can just use the haplotype labels
Removed unnecessary sort operations that were added as an attempt to fix the weird behavior of the program
…arser

Added unit test to check that class has the expected attribtes, handles errors correctly, and has the correct logic
---
On branch refactor
Your branch is up to date with 'origin/refactor'.

Changes to be committed:
	modified:   src/drive/drive.py
	modified:   src/drive/helper_funcs/__init__.py
	new file:   src/drive/helper_funcs/generate_random_filename.py

Added random string to the end of the log filename if the file already
exists:
---
There was a previous race condition where if DRIVE was run on slurm for
multiple jobs but the user was written everything to the same output
directory with the default log filename then the logfile would be
corrupted because multiprocesses would each try to write to it. Now we
append a random 8 character string to the end of the process.
---
On branch refactor
Your branch is up to date with 'origin/refactor'.

Changes to be committed:
	modified:   src/drive/network/cluster/cluster.py

adjusted the gather_members function to use igraph methods to get all of
the members rather than using a for loop.
… networks

Added 2 integration test (covering the no phenotype file and phenotype file branches) that make sure individuals are only appearing in up to 2 networks since their was a recent bug developed with this.
jtb324 added 4 commits April 30, 2026 13:26
…raph functions

Updated the _gather_members adn _determine_true_positive_edges functions to use more of the igraph library's built in functionality
Since DRIVE v3.1.0 uses Duckdb instead of the pandas parser, the chunksize flag is now irrelevant and not used. We removed this flag. Now we have a threads flag that the user can set. This flag limits how many threads duckdb will use by default to read in the IBD data durign the analysis.
fixed conflicts between main and this branch
@jtb324
jtb324 merged commit 30eb1a6 into main Apr 30, 2026
5 checks passed
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.

1 participant