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

[MRG] hnn-core integration #232

Merged
merged 111 commits into from Mar 21, 2021

Conversation

blakecaldwell
Copy link
Member

@blakecaldwell blakecaldwell commented Sep 16, 2020

This runs HNN simulations using the code in hnn-core. Significant changes to HNN parameter handling were required along new code to read and write data files coming from hnn_core objects.

Work remaining:

  • Remove model visualization feature from HNN GUI
  • Update installation docs to remove dependencies on pyopengl for model visualization
  • Add hnn-core prerequisite to installation docs
  • Dipole display after running multiple trials is not correct?
  • Add CI testing base functionality
  • Print status messages fromhnn-core to the simulation log window
  • Correct the simulation shown in combobox after a run
  • save_figs = 1 functionality
  • Data can be held in memory after a simulation rather than writing to disk and re-reading.
  • Open view windows (e.g. spike raster plots) without using POpen()
    • Will document the parameters that are used for each view window in docstrings
  • Integrate somatic voltages/current calculation from HNN core

Closes #231

@blakecaldwell blakecaldwell changed the title [WIP]: HNN core integration [WIP] HNN core integration Sep 16, 2020
@blakecaldwell blakecaldwell changed the title [WIP] HNN core integration [WIP] hnn-core integration Sep 16, 2020
@blakecaldwell blakecaldwell force-pushed the hnn_core_v2 branch 2 times, most recently from f096b1c to 43f0dd0 Compare September 16, 2020 14:26
@blakecaldwell blakecaldwell added this to the hnn-core integration milestone Sep 18, 2020
@blakecaldwell blakecaldwell self-assigned this Sep 22, 2020
@blakecaldwell blakecaldwell removed this from the hnn-core integration milestone Sep 22, 2020
@blakecaldwell blakecaldwell force-pushed the hnn_core_v2 branch 18 times, most recently from f2727c0 to d5a0242 Compare October 5, 2020 03:45
@blakecaldwell
Copy link
Member Author

The PR is ready for testing! Optimization doesn't quite work as expected yet. The other known bug is with recording somatic voltages with cores > 1. I'll be sending out an email inviting others to test, but feel free to give it a go if you have everything installed already

@jasmainak
Copy link
Collaborator

jasmainak commented Mar 20, 2021

@blakecaldwell what do you think of the following workflow?

  • we tag the latest commit on master branch on github as v0.1 (or whatever is the appropriate version number). Maybe also create a branch called maint/v0.1
  • we update the installation instructions to use the v0.1 branch/tag
  • then we merge this branch into master

This way, you can have some peace of mind and we can iterate a bit rather than having to work off this huge branch. It's already looking like a great improvement to the code. We can iterate on the master branch then without breaking user workflows and when we make a new release we update the install instructions

@blakecaldwell
Copy link
Member Author

blakecaldwell commented Mar 20, 2021

@jasmainak yeah, I like that. As you say, we can iterate with new PR's and issues. They can be part of jonescompneurolab/hnn too

So the plan would be naming a new release v1.3.2. Since we've been in "production", releases have been > 1.0. I'll create a branch maint/pre-hnn-core to use in the docs. Also, for any potential compatibility issues that come up, this would draw the before/after line.

@blakecaldwell blakecaldwell changed the title [WIP] hnn-core integration [MRG] hnn-core integration Mar 20, 2021
@blakecaldwell
Copy link
Member Author

As discussed above, we are merging this to master to complete testing and debugging. Installation instructions for Mac and Windows on hnn.brown.edu will have people use release v1.3.2 which was created yesterday.

When we are happy with the post-integration code, we will create a new release and update the instructions.

The developers who are already running hnn from a git clone of master would only be affected if they ran a git pull. They probably will know what's going on if they do this and things look different.

@blakecaldwell blakecaldwell merged commit f876500 into jonescompneurolab:master Mar 21, 2021
@jasmainak
Copy link
Collaborator

There seems to be a duplicate set of installation instructions here: https://github.com/jonescompneurolab/hnn/tree/master/installer

Are these relevant any more? Because they are downloading the latest master

@blakecaldwell
Copy link
Member Author

There seems to be a duplicate set of installation instructions here: https://github.com/jonescompneurolab/hnn/tree/master/installer

Are these relevant any more? Because they are downloading the latest master

Ah, yes. I've been keeping the new instructions in a separate branch (the link that I emailed out). Docs in master are the instructions that everyone sees at hnn.brown.edu.

Just created #280 with the new docs that everyone is using for testing.

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.

Integrate hnn-core for running simulations
5 participants