wsphillips / Conductor.jl

Choo-choo
MIT License
60 stars 12 forks source link

Demo STGneuron not working #22

Closed AndreaRamirezH closed 2 years ago

AndreaRamirezH commented 2 years ago

Hi @wsphillips !

I tried to run the STGneuron file in the demo folder, with a freshly downloaded Conductor, and get this error:

WARNING: could not import Conductor.Cationic into Main
ERROR: LoadError: MethodError: no method matching MembranePotential(::Quantity{Int64, 𝐋² 𝐌 𝐈⁻¹ 𝐓⁻³, Unitful.FreeUnits{(mV,), 𝐋² 𝐌 𝐈⁻¹ 𝐓⁻³, nothing}})
Closest candidates are:
  MembranePotential() at ~/.julia/packages/Conductor/idY1O/src/Conductor.jl:95

Stacktrace:
 [1] top-level scope
   @ ~/.julia/dev/Conductor/demo/prinz_kinetics.jl:6
in expression starting at /Users/andrearamirez/.julia/dev/Conductor/demo/prinz_kinetics.jl:6

These are the packages in my environment

Status `~/.julia/dev/Conductor/demo/Project.toml`
  [d194a933] Conductor v0.0.2
  [615f187c] IfElse v0.1.1
  [961ee093] ModelingToolkit v6.4.9
  [1dea7af3] OrdinaryDiffEq v6.7.1
  [91a5bcdd] Plots v1.27.1
  [295af30f] Revise v3.3.3
  [1986cc42] Unitful v1.11.0

and I'm using Julia Version 1.7.0.

The same error shows up when I try to run STGnetwork.jl and hodgkinhuxley.jl. These demos should work straight from the src code, right?

Thanks for your help!

pfcrowe commented 2 years ago

Hi @AndreaRamirezH

I can get STGneuron.jl and hodgkinhuxley.jl both running with the following setup:

Julia v. 1.7.2

Project Conductor v0.0.2 Status~/Documents/GitHub/Conductor.jl/Project.toml [479239e8] Catalyst v10.7.0 [615f187c] IfElse v0.1.1 [1914dd2f] MacroTools v0.5.9 [961ee093] ModelingToolkit v8.5.2 [1dea7af3] OrdinaryDiffEq v6.7.1 [91a5bcdd] Plots v1.27.1 [efcf1570] Setfield v0.8.2 [d1185830] SymbolicUtils v0.19.7 [0c5d862f] Symbolics v4.3.1 [1986cc42] Unitful v1.11.0 [b77e0a4c] InteractiveUtils

I think the critical package(s) here are Symbolics (and SymbolicUtils?). The other discrepancies may be needed to glue things together though (e.g. ModelingToolkit update for compatibility with the preceding?).

I'm getting a different error with STGnetwork.jl - will poke it and see what I can find.

Hopefully this gets you moving though!

Oh and you might get send: Broken pipe at the last step rather than a plot, usually just re-running the script will work.

pfcrowe commented 2 years ago

For STGnetwork.jl I get errors via prinz_synapses.jl calling Gate and EquilibriumPotential. Re ordering the parameter calls might be sufficient.

I've made a related minimal PR #23 for the moment; next trial step is swapping out Soma for CompartmentSystem in STGnetwork.jl but think I will need to look at MTK to make progress.

AndreaRamirezH commented 2 years ago

Hi @pfcrowe

You're right, activating the environment using the Project.toml file inside the demo folder was not giving me those two packages (Symbolics and SymbolicsUtils).
I updated all packages listed in the dependencies (in here: /.julia/dev/Conductor/Project.toml) and it works :)

I guess the question now is, should there be two different Project and Manifest files? Why are there Manifest and Project files in the demo?

Both STGneuron.jl and hodgkinhuxley.jl work fine, but I get the same error you mention with STGnetwork.

ERROR: LoadError: MethodError: no method matching Conductor.GatingVariable(::Conductor.GateVarForm, ::Symbol, ::Num, ::Num, ::Int64)
Closest candidates are:
  Conductor.GatingVariable(::Any, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any) at ~/.julia/dev/Conductor/src/gates.jl:16
  Conductor.GatingVariable(::Conductor.GateVarForm, ::Num, ::Num, ::Real; name) at ~/.julia/dev/Conductor/src/gates.jl:27
  Conductor.GatingVariable(::Num, ::Num, ::Num, ::Num, ::Num, ::Real, ::Conductor.GateVarType) at ~/.julia/dev/Conductor/src/gates.jl:16
  ...
Stacktrace:
 [1] top-level scope
   @ ~/.julia/dev/Conductor/demo/prinz_synapses.jl:3
in expression starting at /Users/andrearamirez/.julia/dev/Conductor/demo/prinz_synapses.jl:3

Thanks for your help!!

wsphillips commented 2 years ago

Hey sorry for some reason I didn't get a ping when you opened this issue.

First question: Are you both working with the latest tagged version on the registry or the latest commit to main?

Networks are definitely broken on the main branch at the moment because I simply haven't finished implementing NetworkSystem in the same manner as CompartmentSystem and ConductanceSystem

wsphillips commented 2 years ago

Yeah ok. The problem is you're probably both using package/demo files that are out of sync. This is partly because I made some mistakes with the Project/Manifest files.

The tagged version of Conductor.jl on the public registry is older, but should work for ALL demos. This is the version of Conductor.jl you get if you ]add Conductor.jl. (v0.0.2)

If you dev Conductor.jl you will get the latest commit on the main branch, which definitely has some broken bits!

So first, be careful to distinguish whether you are running the v0.0.2 tag or the latest commit of main. I just pushed a version bump to main to make this more obvious on fresh installations. Now if you accidentally dev Conductor.jl without checking out the stable version, it will display version 0.0.3 in the package manager.

If you choose to dev Conductor and you want to run all the demo files, you will have to change to the v0.0.2 tag in the development directory with git checkout v0.0.2 (e.g. inside the .julia/dev/Conductor.jl by default)

If you add Conductor it will install v0.0.2 by default at the moment.

When running the demo, it does appear there's some weirdness because of version changes since I tagged the repo, but you can try the following (I've got this to work on my end):

Once you're sure you have Conductor v0.0.2 installed, go to the demo directory. If you used add Conductor, it might be easier to clone a copy of Conductor.jl to an easily reached directory (and remember to checkout v0.0.2!)

Then, in the demo directory:

  1. julia --project=.
  2. (in package mode) instantiate
  3. (in package mode) up
  4. Run the demo script you want while in the project environment. Best if you go line by line from an editor/IDE

Note: Right now you'll need to up to fix some compatibility issues. This has a side effect of bumping up the version of MTK, which changes the plot recipes. Therefore, to plot the output by variable name, you'll need to use the kwarg vars=[Vβ‚˜] (or similar) in most cases.

Finally, if you're planning to contribute to development, note that the instructions above use the last stable version to get the demos working. However, a lot has changed since then. If you're working with the latest commit on the main branch, the single neuron examples should work but anything with synapses should NOT work right now. That's because of the comment I made earlier:

Networks are definitely broken on the main branch at the moment because I simply haven't finished implementing https://github.com/wsphillips/Conductor.jl/pull/18 in the same manner as CompartmentSystem and ConductanceSystem

wsphillips commented 2 years ago

In STGNetwork.jl using v0.0.2 there's a missing import Unitful.nS. Otherwise, it works using instructions above.

AndreaRamirezH commented 2 years ago

Hi @wsphillips

Yup, that was it. I was getting the main branch when doing dev, indeed.

Thanks for clarifying the instructions to get to the stable v0.0.2. I was trying to simply run the single neurons so this should be enough. Will be looking forward to testing the network when it's ready! ;)

pfcrowe commented 2 years ago

Thanks @wsphillips that was also my case - your instructions were accurate throughout including for the plotting variable.

Wrt development, would you be interested in mentoring J/GSoC this year?

pfcrowe commented 2 years ago

hey @wsphillips i thought to bump this and will follow up with an email - not sure if you are getting pinged as you mentioned. I'm interested in whether or not conductor.jl would be suited to a j/gsoc project - I guess a first question from my side is would this be feasible for you over the relevant period, assuming 350h over min 10 max 22 weeks (targeting 15w?).

pfcrowe commented 2 years ago

relatedly @ChrisRackauckas is Conductor.jl a sufficient priority within the Julia (SciML/NumFOCUS?) ecosystem to merit competitiveness for j/gsoc (this round)?

pfcrowe commented 2 years ago

sorry for hijacking this thread πŸ˜„

ChrisRackauckas commented 2 years ago

I'd be happy to see it move to SciML and make it a GSoC project, and I could help mentor though it would need @wsphillips involved too.

ChrisRackauckas commented 2 years ago

(BTW, remember that SciML GSoC is through NumFOCUS, not JuliaLang)

wsphillips commented 2 years ago

Hi Peter, I'm open to the possibility of mentoring a GSoC project. Aside from an application (see guidelines), we'd like to see a non-trivial trial PR for Conductor.jl.

Given that you now have the stable tagged version of Conductor.jl working, you can explore it a bit to get a rough idea of how things work. Once you feel comfortable, bump up to the latest commit on the development branch. You'll find that there's some changes happening in the structure of the package. Not everything works or is finished yet, but you ought to be able to at least simulate single neurons.

As you're getting familiar with the code base, you may find it helpful to also cross-reference the source for ModelingToolkit.jl itself, as well as Catalyst.jl.

You can take a stab at an open issue/PR or propose something yourself. If you want to bounce ideas off of me, you can email me or ping me on Slack.

Since both @pfcrowe and @AndreaRamirezH seem to have demos running now, I'm going to close this issue. GSoC discussion we can do via other channels.