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

SSML Models Implementation #38

Closed
wants to merge 35 commits into from
Closed

SSML Models Implementation #38

wants to merge 35 commits into from

Conversation

stompsjo
Copy link
Collaborator

This PR introduces much of the code used for my research comparing several different semi-supervised machine learning models (SSML). I wrote and used these models quite separate from how I have used RadClass in the past (i.e. I ran experimental data through RadClass and H0 to get observations I could use for training and testing ML models separately in a downstream analysis).

This raises a design question regarding how these models and scripts should be integrated into RadClass. Should they be made into a post_analysis object? Should they remain separate, thereby making the user have a script that both runs RadClass for data processing and SSML models for training and testing?

  • Another note that popped into my head: should I include a .load method for each model class? This may remove one manual step a user would have to take to use joblib for loading a written model and storing as a model class.

@stompsjo stompsjo requested a review from gonuke August 15, 2022 14:24
@stompsjo stompsjo self-assigned this Aug 15, 2022
@stompsjo
Copy link
Collaborator Author

One problem to note: creating tests for neural networks has been tricky. Ideally, these tests should not take several minutes to train and test a method/model. But if we want meaningful test-cases, we will want to test meaningful neural network architectures (which take longer to train).

I will be thinking of a compromise...

@stompsjo stompsjo marked this pull request as ready for review September 16, 2022 20:13
@stompsjo stompsjo changed the title [Draft] SSML Models Implementation SSML Models Implementation Sep 16, 2022
@stompsjo
Copy link
Collaborator Author

The most recent commit changes each fresh_start method for each ML model to use the parent class train method rather than copying its logic outright. This should help to make each class concise and more comprehensible as a whole.

@gonuke
Copy link
Member

gonuke commented Oct 28, 2022

Split into more PRs?

"Y", "Z"]
jobs = alph[:n]

fig, axes = plt.subplots(n, n, figsize=(15, 15))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
fig, axes = plt.subplots(n, n, figsize=(15, 15))
fig, axes = plt.subplots(n, n, figsize=(3*n, 3*n))

@coveralls
Copy link

coveralls commented Oct 31, 2022

Pull Request Test Coverage Report for Build 3363647923

  • 533 of 534 (99.81%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 99.868%

Changes Missing Coverage Covered Lines Changed/Added Lines %
models/SSML/ShadowCNN.py 142 143 99.3%
Totals Coverage Status
Change from base Build 3348444582: -0.1%
Covered Lines: 754
Relevant Lines: 755

💛 - Coveralls

@stompsjo
Copy link
Collaborator Author

This PR has been split into #41, #42, #44, and #45

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.

3 participants