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

added additional code to output intermediate modeling results in main.py #128

Merged
merged 9 commits into from
Nov 22, 2024

Conversation

ejiawustl
Copy link
Collaborator

No description provided.

@ejiawustl ejiawustl requested a review from cmatKhan November 21, 2024 20:45
yeastdnnexplorer/__main__.py Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
os.makedirs(bootstrap_results_dir, exist_ok=True)

# Save "all" results
ci_dict_all = lasso_res["all"]["ci_dict"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not bother outputing the alphas.

There is repeated code here. The only thing that is different here is 'all' or 'top10'. DRY

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure the names of the output tables are the same. since it is going into args.output_dirpath/<response_tf>/, you do not need to append the response_tf to the filename

yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
yeastdnnexplorer/__main__.py Outdated Show resolved Hide resolved
ejiawustl and others added 8 commits November 21, 2024 13:14
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
Co-authored-by: Chase Mateusiak <chasem@wustl.edu>
…trap models, and updated the interactor_workflow function in main.py to accurately return a single directory with all intermediate and final results
"sig_coefs": sig_coef_dict,
"predictors": X,
"response": y,
"classes": stratification_classes,
}
# this adds the bootstrap lasso outputs so that we can create our own CIs
if method == "bootstrap_lassocv":
Copy link
Member

@cmatKhan cmatKhan Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why the output typing needed to change -- it doesn't look like it did.

We'll make this an issue for later -- I think it is probably better to return this key no matter what, and fill it with None when the method isn't bootstrap_lasso.

If you change output in the future, do adjust the docstring :return: documentation, too

# Save the results from bootstrapping for further analysis
if args.method == "bootstrap_lassocv":
# Iterate through "all" and "top"
for suffix, lasso_results in lasso_res.items():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good!

Copy link
Member

@cmatKhan cmatKhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok -- This looks good. I didn't pull it to run it -- I trust you did.

good work -- thank you for making the revisions

@cmatKhan cmatKhan merged commit 203c2d1 into BrentLab:dev Nov 22, 2024
6 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