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

Omics Integrator 2 Testing Error #182

Merged
merged 10 commits into from
Sep 22, 2024
Merged

Conversation

ntalluri
Copy link
Collaborator

@ntalluri ntalluri commented Sep 3, 2024

No description provided.

@ntalluri
Copy link
Collaborator Author

ntalluri commented Sep 4, 2024

There is now an error with Aggregate per algorithm for the ml step. It only runs when for algorithms with multiple parameter combinations chosen; however, some of those parameter combinations can lead to outputs that are still nothing, which can lead to the bug error from #117

@ntalluri
Copy link
Collaborator Author

ntalluri commented Sep 4, 2024

The issue with the different headers in OI2 is nondeterministic. Although the code functions as intended, the corrupted data happens randomly, causing the ML option aggregate_per_algorithm to fail unpredictably if we keep the default as true.

@agitter
Copy link
Collaborator

agitter commented Sep 4, 2024

Can you tell whether the non-determisitic behavior is the order of 'cost' and 'in_solution' or the presence of those columns? In #133 (comment) I speculated that perhpas the order of those two columns is allowed to be in either order, in which case our quick fix here could sort the columns as long as we get the correct number of columns.

@ntalluri
Copy link
Collaborator Author

ntalluri commented Sep 4, 2024

I can check for the presence of the columns and order. I agree with the idea of sorting the columns as long as the correct number of columns and correct header names exist.

Every time I rerun SPRAS, it seems like the reordering happens at random when all the columns are present and is not specific to the dataset. With the current parameters in the config file, I haven't seen the missing in_solution column yet.

test/parse-outputs/test_parse_outputs.py Show resolved Hide resolved
spras/omicsintegrator2.py Outdated Show resolved Hide resolved
@ntalluri
Copy link
Collaborator Author

Ready for final review

@agitter agitter merged commit 7b07916 into Reed-CompBio:master Sep 22, 2024
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.

2 participants