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

Updates for OpenWorm 0.7 release #102

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Conversation

slarson
Copy link
Member

@slarson slarson commented Jan 5, 2018

Hi guys,
For the version of the OpenWorm dockerfile, I'd like to incorporate David's changes. This will allow us to have the dockerfile point at the CElegansNeuroML repo and therefore centralize some of the many versions that have been floating around. Please review and let's merge ASAP.

Thanks,
Stephen

lungd and others added 21 commits August 23, 2017 12:03
E.g. data_reader, config_package, input_list, conns_to_include
So the optimizer can also add new parameters instead of only changing the magnitude. Therefore the unit needs to be known.
parameters['AVBR_to_DB1_elec_syn_gbase'] = {'value':'0.1', 'unit':'nS'}
self.volts['MDL01/0/GenericMuscleCell/v'] = [...]
Check if type of param is dict or list (new or old version).
Use multiprocessing.pool instead of parallelpython
This reverts commit 87bf234.
 Conflicts:
	CElegans/pythonScripts/c302/tune/C302Simulation.py
Only override/add param by a regex unless there is a param defined without a regex.
new models, other changes
Copy link
Member

@pgleeson pgleeson left a comment

Choose a reason for hiding this comment

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

Some feedback

@@ -7377,4 +7376,20 @@ vm2pL , vm2pR ,4,electrical
vm2pR , VC03 ,2,electrical
vm2pR , um1pR ,4,electrical
vm2pR , vm1pR ,4,electrical
vm2pR , vm2pL ,4,electrical
Copy link
Member

Choose a reason for hiding this comment

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

@lungd Why are these changes being added to herm_full_edgelist.csv?

Surely this should remain identical to https://github.com/openworm/PyOpenWorm/blob/dev/OpenWormData/aux_data/herm_full_edgelist.csv (or the reason for the change documented...)

Copy link
Member Author

Choose a reason for hiding this comment

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

@pgleeson -- Is there a way to do this via code? @lungd thinks we may have to add to the API to allow this?

min_constraints = []
max_constraints = []

parameters['AVBL_to_AVBR_exc_syn_conductance'] = {'default_unit': 'nS'}
Copy link
Member

Choose a reason for hiding this comment

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

@lungd Are these necessary here? Fine to keep them as long as they're not temp additions... Have you checked to see if some of the other optimisation examples in these (tuning) files still work?

@pgleeson
Copy link
Member

@lungd I've manually added all these changes on the master branch, apart from the changes to the tuner code (which I still need to retest on mine; but it would be good if you double check if these are necessary/correct in your changes) and the additions to herm_full_edgelist.csv (see above).

Can you merge these into yours and test? They'll require the latest sibernetic_c302 (here).

Btw, all of my updates are working in docker in this branch: https://github.com/openworm/OpenWorm/tree/development_c302 (if you want to try them out too @slarson)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants