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

Channel recovery #108

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

Channel recovery #108

wants to merge 7 commits into from

Conversation

TheLemonPig
Copy link
Collaborator

Channel Recovery training:

  1. Adding a recover_channels parameter to the autoencoder training
  2. Adding a recover_channels parameter to the gan training
  3. Train AE and GAN with this same parameter adjustment

Add `recover_channels` to `opt`
Adding `recover_channels` to default system inputs for the autoencoder and to the helper menu
Make sure both AE and GAN have the added parameter
Add `recover_channels` from `opt` into the GAN and AE configuration files
@TheLemonPig
Copy link
Collaborator Author

Is there some way to check the Autoencoder's configuration while you are inside the GAN training? The only reason to add any mention of channel recovery in the GAN training is just to make sure that the user knows that the AE they are using was trained to recover channels (i.e. to make sure they choose the right AE for their GAN training)

Add `recover_channels` to `opt`
Added channel recovery code to AE training
Update system inputs to match the update usage of the `recover_channels` parameter
Copy link
Collaborator

@chadcwilliams chadcwilliams left a comment

Choose a reason for hiding this comment

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

I am delaying the PR for two reasons.

  1. The dev branch is currently EEG-GAN v2.0 as will be deployed, so at this point it should not be modified. I will work on deploying it soon so that the dev branch is again open for changes.

  2. You mentioned in slack that these changes are not yet tested. I believe it would be best to ensure that this works on your branch before we merge into the code. This would include making sure that all past functionality still works (i.e., your changes did not effect the package when not doing channel recovery).

@TheLemonPig
Copy link
Collaborator Author

Hi @chadcwilliams sounds good. I completely understand. I wanted to create this as a draft PR so that you guys could see the work as it was being done. I wouldn't want to make it a proper PR until I can confirm it works. I think that is a great idea to also make sure that all previous functionality is not broken by the added features.

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