Skip to content

Created reconstruction method#364

Merged
mihaic merged 39 commits intobrainiak:masterfrom
dhuberdeau:invenc
Nov 14, 2019
Merged

Created reconstruction method#364
mihaic merged 39 commits intobrainiak:masterfrom
dhuberdeau:invenc

Conversation

@dhuberdeau
Copy link
Copy Markdown
Contributor

Added a reconstruct folder, including Inverted Encoding Models (1.Brouwer, G. J. & Heeger, D. J. Decoding and Reconstructing Color from Responses in Human Visual Cortex. J. Neurosci. 29, 13992–14003 (2009).).

TODO: Complete test functions and example function for the inverted encoding model method.

Added a reconstruct folder, including Inverted Encoding Models (1.Brouwer, G. J. & Heeger, D. J. Decoding and Reconstructing Color from Responses in Human Visual Cortex. J. Neurosci. 29, 13992–14003 (2009).).

TODO: Complete test functions and example function for the inverted encoding model method.
@buildbot-princeton
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

1 similar comment
@buildbot-princeton
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@mihaic
Copy link
Copy Markdown
Member

mihaic commented Apr 26, 2018

Jenkins, add to whitelist.

@mihaic
Copy link
Copy Markdown
Member

mihaic commented May 7, 2018

@dhuberdeau, are you making progress? Please reach out if you need help.

@dhuberdeau
Copy link
Copy Markdown
Contributor Author

@mihaic, thanks for reaching out. Yes, getting to these changes this week. I'm sure I'll have some questions, so I'll be in touch.

@dhuberdeau
Copy link
Copy Markdown
Contributor Author

This is a pull request for the Inverted Encoding Model (IEM) implementation. It creates a new folder, reconstruct, to which additional methods of reconstruction can be added.

This particular IEM implementation is for 1-D data only. Future development should add multi-dimensional reconstruction capabilities.

I'm not sure who would be appropriate as a reviewer. Perhaps Qi?

-David

@mihaic
Copy link
Copy Markdown
Member

mihaic commented May 16, 2018

@dhuberdeau, do you mean @qihongl?

@dhuberdeau
Copy link
Copy Markdown
Contributor Author

Hi @mihaic: Yes. I heard through the grapevine that @qihongl might be a good reviewer for this. I haven't asked him yet, though. Do you have any suggestions?

@mihaic
Copy link
Copy Markdown
Member

mihaic commented May 16, 2018

Let's hear from him first. @qihongl?

@qihongl
Copy link
Copy Markdown
Member

qihongl commented May 18, 2018

Hi, Folks! Sorry for the delayed response! I will start working on this by the end of next week!

@dhuberdeau
Copy link
Copy Markdown
Contributor Author

Thanks Qihong! I'll be available to make changes as you see fit. Don't hold back..

@qihongl
Copy link
Copy Markdown
Member

qihongl commented May 18, 2018

@dhuberdeau @mihaic
I haven't look at the detail, but just a quick question: should we follow the standard code comment style?
e.g., SRM: https://github.com/brainiak/brainiak/blob/master/brainiak/funcalign/srm.py

@mihaic
Copy link
Copy Markdown
Member

mihaic commented May 18, 2018

@qihongl, indeed. We actually formalized the coding standards in the docs:
http://brainiak.org/docs/contributing.html#standards

@qihongl
Copy link
Copy Markdown
Member

qihongl commented May 18, 2018

@mihaic Yep. Thanks for the reference!
@dhuberdeau Can you add comments according to the style guide by @mihaic ? Thanks!

@mihaic
Copy link
Copy Markdown
Member

mihaic commented May 29, 2018

@qihongl, any progress with the review?

@dhuberdeau, the "WIP" part of the PR title is not necessary anymore, right?

@qihongl
Copy link
Copy Markdown
Member

qihongl commented May 29, 2018

@mihaic Oops, I haven't started. I was waiting for the documentation and then I forgot. I will start working on it today or tomorrow!

@dhuberdeau Have you started documenting the code? Any documentation would be really helpful.

@dhuberdeau
Copy link
Copy Markdown
Contributor Author

Hi @mihaic and @qihongl, I'll add in proper documentation and do another commit. I'll let you know when that is done and ready for review. -David

@qihongl
Copy link
Copy Markdown
Member

qihongl commented May 30, 2018

@dhuberdeau awesome! Thanks!

@qihongl
Copy link
Copy Markdown
Member

qihongl commented Jun 4, 2018

Thanks! @dhuberdeau I will work on that ASAP!

@CameronTEllis
Copy link
Copy Markdown
Contributor

@mihaic Not sure about the Ruby error on Travis. Sounds like it is failing on BRSA?

@vyaivo
Copy link
Copy Markdown
Contributor

vyaivo commented Oct 11, 2019

Hi @qihongl -- I think the issues from your previous review have been resolved, and we should be ready to merge soon. Would you be willing to review this PR again?

@qihongl
Copy link
Copy Markdown
Member

qihongl commented Oct 15, 2019

Sure! Happy to help with this!

@qihongl
Copy link
Copy Markdown
Member

qihongl commented Oct 29, 2019

Sorry about the delay :( I have been preparing for my general/qualification exam and I still need some time. If this is urgent, feel free to ask other folks to review this. And I will try to reply in two weeks.

@vyaivo
Copy link
Copy Markdown
Contributor

vyaivo commented Nov 12, 2019

Hi @qihongl, hope all is going well. I just wanted to check in since it has been about 2 weeks.

@qihongl
Copy link
Copy Markdown
Member

qihongl commented Nov 12, 2019

Ah sorry, I will do my oral presentation tomorrow. I will work on this asap!

Copy link
Copy Markdown
Member

@qihongl qihongl left a comment

Choose a reason for hiding this comment

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

some minor comments

@qihongl
Copy link
Copy Markdown
Member

qihongl commented Nov 13, 2019

I think the update is great. And the notebook examples make a lot of sense. I just had some really minor comments. Thank you very much!

@vyaivo
Copy link
Copy Markdown
Contributor

vyaivo commented Nov 14, 2019

Thanks @qihongl! I addressed your comments and removed another outdated example file (in addition to the ones you flagged). I think we're good to merge.

@mihaic mihaic merged commit ad3c527 into brainiak:master Nov 14, 2019
@mihaic
Copy link
Copy Markdown
Member

mihaic commented Nov 14, 2019

Thanks a lot for your contributions, @dhuberdeau and @vyaivo! Looking forward to expanding this new reconstruct package.

mihaic added a commit to mihaic/brainiak that referenced this pull request Nov 14, 2019
With our current workflow, we are not crediting all authors. See for
example PR brainiak#364, where Vy Vo is not credited.
@mihaic mihaic mentioned this pull request Nov 14, 2019
gdoubleyew pushed a commit that referenced this pull request Nov 25, 2019
With our current workflow, we are not crediting all authors. See for
example PR #364, where Vy Vo is not credited.
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.

6 participants