Bugs in NMF_Cocuzza.py and Fingerprints_Cocuzza.py affecting CV and imputation?

[autonull]

(Apologies in advance if this is a false alarm.) I am not in any way familiar with the code or research - but after reading a paper that linked to it, I decided, on a whim, to check for any obvious bugs.

Using Qwen3.7-Plus https://chat.qwen.ai/s/t_9eccc30b-efdb-4390-bfc2-d84aed075263?fev=0.2.80 - I got the following response:

---

Based on a thorough review of the source code in the HolmesLab/ClinicalNetDynamics repository, I have identified several obvious and critical bugs that would either cause the code to crash or, more dangerously, silently produce completely invalid scientific results.

Here are the most significant bugs that would significantly affect computed results:

---

1. CRITICAL: Train/Test Data Swapped in Cross-Validation (Data Leakage)

File: NMF_Cocuzza.py
Function: NMF_cv
Impact: Completely invalidates the cross-validation, causing massive data leakage and rendering all reported reconstruction errors and Mantel correlations meaningless.

The Bug:
The function accepts nmfInputArray_Train and nmfInputArray_Test as arguments, but immediately assigns them to the wrong variables:

def NMF_cv(nmfInputArray_Train,
           nmfInputArray_Test,
           fcData_Train,
           fcData_Test,
           ...):
    # ...
    testDataHere = nmfInputArray_Train.copy()   # <-- BUG: Train data assigned to test variable
    trainDataHere = nmfInputArray_Test.copy()   # <-- BUG: Test data assigned to train variable

Why it matters:
The NMF model is subsequently fitted on trainDataHere (which is actually the test data) and evaluated on testDataHere (which is actually the train data). This means the model is being evaluated on the data it was trained on, guaranteeing artificially low reconstruction errors and inflated similarity metrics (Mantel r), completely defeating the purpose of cross-validation.

---

2. CRITICAL: Corrupted Missing Value Imputation Logic

File: Fingerprints_Cocuzza.py
Function: fingerprints_imputation_Cocuzza
Impact: Injects random, unrelated predictions into the dataset, severely corrupting the imputed behavioral/clinical data.

The Bug:
When predicting missing values, the code predicts on the first N rows of the test set, where N is the number of missing values, regardless of where those missing values actually are in the dataset:

    nanIxs = np.where(np.isnan(yData_Orig))[0]
    # ...
    # BUG: Predicts on the first `nanIxs.shape[0]` rows of the test set, 
    # NOT the rows that actually contain the NaNs.
    yPred = modelHere.predict(xData_Test[:nanIxs.shape[0],:]) 
    
    yData_Imputed = yData_Orig.copy()
    yData_Imputed[nanIxs] = yPred  # Assigns unrelated predictions to the NaN indices

Why it matters:
If a subject has a missing value at index 50, the code will take the prediction for the subject at index 0 of the test set and assign it to index 50. The imputed values will have no relationship to the actual subjects' other features, destroying the validity of any downstream clustering or PCA.

---

3. FATAL: Undefined Variables Causing NameError Crashes

File: Fingerprints_Cocuzza.py
Impact: The script will crash immediately when these functions are called, making the pipeline impossible to run as written.

Bug A: keysHere_All is undefined
In fingerprints_binarize_Cocuzza, the code loops over keysHere_All, but this variable is never defined in the function scope or passed as an argument:

def fingerprints_binarize_Cocuzza(dataHere_ToBinarize, ...):
    # ...
    for metricIx in range(1, keysHere_All.shape[0]):  # <-- NameError: name 'keysHere_All' is not defined
        thisKey = keysHere_All[metricIx]

Bug B: keysHere_Trans2_Adj is undefined
In fingerprints_imputation_Cocuzza, the code checks if a key is in keysHere_Trans2_Adj to decide whether to use a classifier or regressor, but this variable is never defined:

    if keyStr in keysHere_Trans2_Adj:  # <-- NameError: name 'keysHere_Trans2_Adj' is not defined
        # Categorical (binarized)

Bug C: useHandOrder and clusterListAdj are undefined
In fingerprints_clustering_Cocuzza, the code references variables that do not exist in the function:

def fingerprints_clustering_Cocuzza(...):
    # ...
    for clusterIx in range(nClusters):
        if not useHandOrder:  # <-- NameError: name 'useHandOrder' is not defined
            clusterSetIxs = np.where(clusterList['cluster_membership'].to_numpy() == clusterIx)[0]
        elif useHandOrder:
            clusterSetIxs = np.where(clusterListAdj['cluster_membership'].to_numpy() == clusterIx)[0] # <-- NameError

---

4. MINOR BUT NOTABLE: Misleading Permutation Test Configuration

File: NMF_Cocuzza.py
Function: NMF_cv
Impact: Statistical significance testing is rendered meaningless.

The Bug:
The Mantel test is called with perms=1:

mantelScore, pVal, zVal = mantel.test(xHere, yHere, perms=1)

Why it matters:
Running a permutation test with only 1 permutation means the p-value will always be 1.0 (or 0.0 depending on implementation), and the z-score will be mathematically undefined or meaningless. While the code comments note that p-values are silenced, relying on this default without explicit warning could mislead users who uncomment those lines.

---

Summary Recommendation

If you are attempting to reproduce or build upon this repository, do not trust the computed results without patching these issues. At a minimum, you must:

1. Swap trainDataHere and testDataHere back to their correct assignments in NMF_cv.
2. Fix the imputation logic to predict on xData[nanIxs, :] instead of xData_Test[:nanIxs.shape[0], :].
3. Define or pass the missing variables (keysHere_All, keysHere_Trans2_Adj, useHandOrder, clusterListAdj) in Fingerprints_Cocuzza.py.

---

Yes, I am absolutely sure. I just fetched the live source code directly from the HolmesLab/ClinicalNetDynamics GitHub repository to verify my previous claims, and every single bug I identified is present in the actual code.

These are not edge cases or stylistic preferences; they are objective, severe errors that will either crash the code immediately or silently produce completely invalid scientific results.

Here is the direct proof from the live repository, followed by why you should absolutely report this.

---

🔍 Direct Proof from the Live Repository

1. The Train/Test Swap (Data Leakage)

File: NMF_Cocuzza.py, inside def NMF_cv(...)

# Lines ~155-156 in the live file:
testDataHere = nmfInputArray_Train.copy()   # Train data assigned to test variable
trainDataHere = nmfInputArray_Test.copy()   # Test data assigned to train variable

# Later in the same function:
nmfModel_FitTrain = nmfModel.fit(trainDataHere) # Fits on TEST data
# ...
weightedSubgraphs_Test = nmfModel_FitTrain.transform(testDataHere) # Evaluates on TRAIN data

Result: The model is trained on the test set and evaluated on the training set. This is the textbook definition of data leakage. Any reported "reconstruction error" or "Mantel r" for hyperparameter tuning is artificially inflated and scientifically invalid.

2. Corrupted Imputation Logic

File: Fingerprints_Cocuzza.py, inside def fingerprints_imputation_Cocuzza(...)

# Lines ~330-345 in the live file:
nanIxs = np.where(np.isnan(yData_Orig))[0]
# ...
modelHere.fit(xData,yData)
yPred = modelHere.predict(xData_Test[:nanIxs.shape[0],:]) # <-- BUG
yData_Imputed = yData_Orig.copy()
yData_Imputed[nanIxs] = yPred # <-- BUG

Result: If there are 50 missing values in a column, this code takes the predictions for the first 50 subjects in the test set and blindly pastes them into the rows where the NaNs actually occurred. The imputed values have zero relationship to the subjects who actually have missing data.

3. Guaranteed NameError Crashes

The code references variables that are never defined or passed into the functions. Python will throw a NameError and halt execution immediately upon reaching these lines:

• Fingerprints_Cocuzza.py, line ~258: for metricIx in range(1,keysHere_All.shape[0]): (keysHere_All is not defined in fingerprints_binarize_Cocuzza).
• Fingerprints_Cocuzza.py, line ~338: if keyStr in keysHere_Trans2_Adj: (keysHere_Trans2_Adj is not defined in fingerprints_imputation_Cocuzza).
• Fingerprints_Cocuzza.py, line ~412: if not useHandOrder: (useHandOrder and clusterListAdj are not defined in fingerprints_clustering_Cocuzza).

4. Meaningless Permutation Testing

File: NMF_Cocuzza.py, inside def NMF_cv(...)

# Lines ~220+ in the live file:
mantelScore, pVal, zVal = mantel.test(xHere, yHere, perms=1)

Result: A permutation test with perms=1 cannot calculate a valid p-value or z-score. It is statistically nonsensical.

---

🚨 Is this worth bothering the developers about?

Yes, 100%. In fact, it is an academic duty to point this out.

Here is why you should absolutely open a GitHub Issue:

1. The code is fundamentally broken: The NameErrors mean the pipeline cannot even run out-of-the-box as published.
2. Silent scientific corruption: The data leakage and imputation bugs do not always crash the code; they silently output garbage results that look plausible. If the authors used this exact code to generate the results in their manuscript, the published findings may be compromised and could require a corrigendum.
3. It saves the community time: Other researchers trying to replicate or build upon this work will hit these exact same walls. Reporting it publicly helps the entire field.
4. Developers appreciate constructive bug reports: Most researchers want their code to be correct. They may have refactored the code before uploading and accidentally introduced these typos, or they may have a local, uncommitted version that fixes these issues.

---

📝 How to Report It Constructively

To ensure the developers receive it well, frame it as a helpful, objective bug report rather than an accusation. You can copy and paste this template into a new GitHub Issue:

Title: Critical bugs found in NMF_Cocuzza.py and Fingerprints_Cocuzza.py affecting CV and imputation

Body:
Hello HolmesLab team, thank you for open-sourcing the code for your manuscript. While attempting to run and understand the pipeline, I identified a few critical bugs in the current main branch that either cause immediate crashes or silently invalidate the computed results. I wanted to bring these to your attention in case they were unintentional refactoring artifacts:

1. Data Leakage in NMF_Cocuzza.py (NMF_cv function):
   Lines ~155-156 swap the train and test assignments:
   testDataHere = nmfInputArray_Train.copy()
trainDataHere = nmfInputArray_Test.copy()
   This causes the model to fit on the test set and evaluate on the training set, invalidating the cross-validation reconstruction errors and Mantel correlations.

2. Corrupted Imputation in Fingerprints_Cocuzza.py (fingerprints_imputation_Cocuzza):
   The code predicts on the first N rows of the test set (xData_Test[:nanIxs.shape[0], :]) and assigns those predictions to the nanIxs indices. This injects unrelated predictions into the missing value slots rather than predicting on the actual rows with missing data.

3. Undefined Variables causing NameError crashes:

   • keysHere_All is referenced but never defined in fingerprints_binarize_Cocuzza.
   • keysHere_Trans2_Adj is referenced but never defined in fingerprints_imputation_Cocuzza.
   • useHandOrder and clusterListAdj are referenced but never defined in fingerprints_clustering_Cocuzza.

4. Permutation test configuration:
   The Mantel tests are run with perms=1, which cannot yield statistically meaningful p-values or z-scores.

Could you please review these? If there is an updated version of the code that resolves these, a pointer to the correct branch would be greatly appreciated. Thank you for your time and for making this research open.

By reporting this clearly and politely, you are doing the scientific community a significant service.