openworm / org.geppetto

Geppetto is an open-source platform to build web-based applications to visualize and simulate neuroscience data and models.
http://geppetto.org
Other
209 stars 50 forks source link

Review Sprint 33 #226

Closed tarelli closed 9 years ago

tarelli commented 10 years ago

@mlolson

@jrmartin

tarelli commented 10 years ago
mlolson commented 10 years ago

@tarelli When does it happen?

tarelli commented 10 years ago

@mlolson haven't yet identified "when" but it's the second time I open the console and it's there

mlolson commented 10 years ago

Ok I'm not seeing that but I'm pulling everything down and rebuilding the project

mlolson commented 10 years ago

I'll work on all of these except the first.

jrmartin commented 10 years ago

@mlolson The Webgl failing is already fixed too. I will do the first one.

jrmartin commented 10 years ago

I added another issue to the card, sometimes I noticed that the Loading Spinner won't go away after simulation is loaded.

tarelli commented 10 years ago

@jrmartin when that happens (spinner doesn't go away) do you have the Invariant violation error in the console (see last item)?

jrmartin commented 10 years ago

Yes, that error is displayed in the browser debug console.

mlolson commented 10 years ago

@jrmartin When I try to load the Scatter 3D simulation I get the "Cannot read property '0' of undefined " error. Are you familiar with this error?

screen shot 2014-09-10 at 11 20 45 am

jrmartin commented 10 years ago

@tarelli The c302 simulation has the variables named like this c302.electrical.SimulationTree.ADAR.0, using ADAR[0] would make more sense, but I didn't create that script so I'm not sure which one is the correct way

jrmartin commented 10 years ago

@mlolson It's a problem with the variable not existing, you can transfer that issue to me since the comment above relating C302 simulation is related to this one.

mlolson commented 10 years ago

Ok cool, this looked like something you were familiar with

jrmartin commented 10 years ago

@tarelli For the issue of value including unit, it's because Purkinje model tree has dummy data. The model tree is populated with random fields from the .nml, and value happened to be the field "erev" which in model tree is shown as erev="137.5262 mV". Issue should not be issue once model tree is populated accurately.

Print method is missing for model tree nodes, this is client side.

tarelli commented 10 years ago

@jrmartin about the .0 vs [0] you are right it comes from LEMS internal representation (@adrianq see the LEMS file and you will find "ADAL/0/generic_iaf_cell/v" which translates to ADAL.0.generic_iaf_cell.v), in that case the question becomes where does the [0] come from?

tarelli commented 10 years ago

@mlolson this is still happening in my dev env after updating screen shot 2014-09-11 at 17 28 17

mlolson commented 10 years ago

@tarelli Yeah this is some weird chrome rendering issue I think. I thought it was due to a 1px solid transparent border on disabled buttons, but that seems to not be the case. Will investigate further.

jrmartin commented 10 years ago

@tarelli What branch did you get the SPH not restartign problem? It's working fine in dev.

tarelli commented 10 years ago

@mlolson when you have a chance can you check if in the dev branch you can stop and restart the SPH simulation (remember to use the sample from the dev repository), in my local dev environment when I restart it after stopping it the simulation doesn't restart. It works fine in @jrmartin's.

mlolson commented 10 years ago

I'm seeing the same issue

pgleeson commented 10 years ago

See mail on main list re having more time for NeuroML/OpenWorm issues next week... and this should go for this too... But for now...

It's important to note that variables in different LEMS models will be accessed/named in different ways, depending on whether the model contains:

1) A top level single Component being run directly (e.g. https://github.com/NeuroML/NML2_LEMS_Examples/blob/master/Figure8_SBML_LEMS.xml) 2) A simple component in a network/population (e.g. https://github.com/NeuroML/NeuroML2/blob/development/LEMSexamples/LEMS_NML2_Ex9_FN.xml) 3) An abstract (no morphology) cell with membrane potential in a network: https://github.com/NeuroML/NeuroML2/blob/development/LEMSexamples/LEMS_NML2_Ex8_AdEx.xml 4) A single segment cell with v (or other variables like bioPhys1/membraneProperties/kChans/kChan/n/q) in a population: https://github.com/NeuroML/NeuroML2/blob/development/LEMSexamples/LEMS_NML2_Ex5_DetCell.xml 5) A membrane potential on a named segment in a multicompartmental cell: path will be something like: CellGroup_1/0/L5PC/123/v. This can't be used by jLEMS (no multicompartmental cell support) but will be needed when exported to NEURON.

These various strings are constrained in part by what jLEMS needs to be given, rather than a sensible way to represent paths to variables, and may change in future. To help this, I've got a class here: https://github.com/NeuroML/org.neuroml.export/blob/development/src/main/java/org/neuroml/export/LEMSQuantityPath.java which reads all the different forms in and gives a class to query what it means. As the string changes this class will remain and be updated to read the quantity from LEMS. Hope this helps.

jrmartin commented 10 years ago

@tarelli @pgleeson I spent some time trying to make the console accept any paths for the variables, but due to Javascript syntax restriction variables can't be named as numbers (just like java). So c302.electrical.SimulationTree.ADAR.0 is not allowed, not even by the browse's javascript console. It needs to start with a letter, $ or underscore. Maybe if the variable path contains a number we can add an underscore prior to it for Javascript to allow, so "c302.electrical.SimulationTree.ADAR.0" would become "c302.electrical.SimulationTree.ADAR._0" .

tarelli commented 10 years ago

@jrmartin ok, I started working on the neuromlpopulation branch where among other things I'm also following through the design of extracting entities from the NeuroML file so this problem will be also taken care of. The new path will be c302.ADAR.electrical.SimulationTree etc, the magic will happen in the ModelInterpreter/Simulator. @adrianq these are good branches also to start extracting more stuff from the NeuroML files, to work with it the branch exists for core/simulation/model.neuroml.

jrmartin commented 9 years ago

As server side performance goes, it's all good now that duplicate readModel() is gone. There's still some lagging client side, but only when loading simulation through LoadingModal, doesn't happen if loading simulation via command prompt or URL. @mlolson Do you know if react components take a while to disappear and freeze the client during that time? It's my first thought as to why this might be happening, that the loading modal takes a couple seconds to disappear after clicking "Load" and the socket can't send the command to load the simulation until this happens.

mlolson commented 9 years ago

Hmm, react components should be very quick to render, but there might be something else going on. I'm seeing some very slow load times on live.geppetto.org, a bunch of server requests are taking >4 seconds

tarelli commented 9 years ago

@jrmartin I just noticed that line 170 of GeppettoMessageInbound is hit every time the user edits the Simulation URL textbox, as you type. It should only be called when switching to the custom tab I think.

jrmartin commented 9 years ago

@mlolson Thanks for the clarification on React components, those slow load time on live should be gone on dev now. @tarelli That might be crowding socket, saw them but they seemed to go instantaneously, maybe that's the issue. Thanks for tip

jrmartin commented 9 years ago

@tarelli Well that was it, should follow occam's razor when debugging more often instead of thinking is something more complicated. Thanks.

tarelli commented 9 years ago

@jrmartin where did the fix for this go?

jrmartin commented 9 years ago

@tarelli Development branch

tarelli commented 9 years ago

@jrmartin of which bundle? I merged frontend dev into adrianq-neuromlwidget but it was still happening

jrmartin commented 9 years ago

@tarelli Frontned, this it the commit that takes care of it https://github.com/openworm/org.geppetto.frontend/commit/abfecad0b7739bc6940a5992c1b00896204d2218

jrmartin commented 9 years ago

@tarelli I'm working off the adrianq-neuromlwidget, seems the changes weren't merged properly

jrmartin commented 9 years ago

@tarelli I merged the adrianq-neurml and development branches into a branch called devneuromlmerged which is committed. There were too many manual changes I had to make, didn't want to risk merging back to adrianq-neuroml branch without knowing that branch full functionality.

adrianq commented 9 years ago

If Scatter3d is working fine maybe we want to add the simulation back to the Load Simulation scroll down menu...

tarelli commented 9 years ago

Yup, I'm doing that