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

TrainingModule parameter changes #1371

Merged
merged 10 commits into from
Nov 10, 2020

Conversation

AlexanderGetka-cbica
Copy link
Contributor

This PR contains some changes to the GUI and CLI of training module and a few changes to the back-end to just make it work while the bigger changes are pending.

This does not include the (sweeping) structural changes to the TrainingModule.cpp back-end -- this PR mostly introduces the Parameter Object pattern so as to more easily plumb data through (also, C and G optimization options are now tunable).

Those big refactors on the back-end side will come later as a PR, before the addition of the new feature selection approaches. That way, the structural changes with the existing modes will come first, then the additions as a separate PR to make it easier to review.

@AlexanderGetka-cbica
Copy link
Contributor Author

@sarthakpati Did our github permissions change? I can't seem to request reviewers anymore. UI bug, maybe?

@sarthakpati
Copy link
Contributor

@sarthakpati Did our github permissions change? I can't seem to request reviewers anymore. UI bug, maybe?

Try now?

@AlexanderGetka-cbica
Copy link
Contributor Author

@sarthakpati Did our github permissions change? I can't seem to request reviewers anymore. UI bug, maybe?

Try now?

OK, that worked. Thanks!

@sarthakpati
Copy link
Contributor

Hey @AlexanderGetka-cbica @Getkablue: if you are still working on this, could convert to draft? Thanks!

@AlexanderGetka-cbica
Copy link
Contributor Author

This is actually done for now, I just forgot to commit my last change. This can be reviewed.

Copy link
Collaborator

@ashishsingh18 ashishsingh18 left a comment

Choose a reason for hiding this comment

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

Overall looks very good. See a minor comment about params.

@sarthakpati sarthakpati merged commit a00453a into CBICA:master Nov 10, 2020
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.

4 participants