Add support for NEST circuits (and spikes visualization) #31

favreau · 2016-09-21T08:52:24Z

No description provided.

jafyvilla

First round of comments...more are coming, but I think it's better to sit together and discuss.

jafyvilla · 2016-09-21T13:52:04Z

apps/BraynsViewer/BraynsViewer.cpp

@@ -101,6 +97,10 @@ void BraynsViewer::keypress(char key, const Vector2f& where)
        BRAYNS_INFO << "Default renderer activated" << std::endl;
        renderingParams.setRenderer("exobj");
        break;
+    case '7':


We should start thinking about this hotkeys madness... soon we'll need bigger keyboards :)

Agreed! I think the nice way to do it is to create a 'key manager' than can be shared between the UI and Deflect so that we get the same interaction on all displays. TODO then ;-)

jafyvilla · 2016-09-21T13:54:07Z

apps/BraynsViewer/BraynsViewer.cpp

-    size_t ts = _brayns->getParametersManager().getSceneParameters().getTimestamp();
-    if( ts != std::numeric_limits<size_t>::max() )
+    float ts = _brayns->getParametersManager().getSceneParameters().getTimestamp();
+    if( ts != std::numeric_limits< float >::max() )
        ss << " (frame " << ts << ")";


I guess it is a float now because it contains the actual timestamp instead of the frame number. If that's the case, the message should also be changed.

I like the timestamp as a float, rather than a frame as a int. In any simulation, what is interesting is the timestamp.

jafyvilla · 2016-09-21T13:57:48Z

apps/ui/manipulators/FlyingModeManipulator.cpp

@@ -35,6 +35,7 @@ void FlyingModeManipulator::keypress( int32 key )
        {
            const float fwd = _window.getMotionSpeed();
            viewport.translate( dir*fwd, true );
+            std::cout << viewport << std::endl;


jafyvilla · 2016-09-21T14:01:01Z

brayns/Brayns.cpp

        {
            SceneParameters& sceneParameters =
                _parametersManager->getSceneParameters();
            const std::string& transferFunctionFilename =
                sceneParameters.getTransferFunctionFilename();
            if( !transferFunctionFilename.empty() )
            {
-                TransferFunctionLoader transferFunctionLoader;
+                TransferFunctionLoader transferFunctionLoader(
+                    brayns::Vector2f( -92.0915, 49.5497 ));


jafyvilla · 2016-09-21T14:11:47Z

brayns/parameters/GeometryParameters.h

@@ -94,7 +102,7 @@ class GeometryParameters : public AbstractParameters
    float getRadiusMultiplier( ) const { return _radiusMultiplier; }

    /** Radius correction applied to spheres and cylinders.
-     * @param value Radius value. The radius contained in the data source is
+     * @param value Radius vPARAM_SIMULATION_CACHE_FILENAMEalue. The radius contained in the data source is


jafyvilla · 2016-09-21T14:13:42Z

plugins/engines/ospray/render/OSPRayScene.cpp

@@ -731,35 +730,38 @@ void OSPRayScene::commitSimulationData()
        const uint64_t frame = _sceneParameters.getTimestamp();


yes, float...

jafyvilla · 2016-09-26T14:55:48Z

brayns/Brayns.cpp

@@ -99,9 +102,15 @@ struct Brayns::Impl
    {
        GeometryParameters& geometryParameters =
            _parametersManager->getGeometryParameters();
+        if(!geometryParameters.getSimulationCacheFile().empty())


I just noticed that this is already checked inside the function, so we could remove it from here

jafyvilla · 2016-09-26T14:57:44Z

brayns/Brayns.cpp

        if(!geometryParameters.getMorphologyFolder().empty())
            _loadMorphologyFolder();

+        if(!geometryParameters.getNESTCircuit().empty())


jafyvilla · 2016-09-26T15:07:37Z

brayns/Brayns.cpp

+        if( !circuit.empty( ))
+        {
+            size_t nbMaterials;
+            NESTLoader* loader( new NESTLoader( geometryParameters ));


Use a smart ptr here too? Same as others that are typedef'd

Well, no need for a pointer at all... Removed it.

jafyvilla · 2016-10-04T13:05:38Z

brayns/common/vocabulary/spikes.fbs

+namespace zerobuf.data;
+
+// Spikes activated for a given timestamp
+table Spikes


Not used yet?

Nope, but will eventually.

jafyvilla · 2016-10-04T13:06:44Z

brayns/io/NESTLoader.cpp

+    {
+        BRAYNS_PROGRESS( gid, _frameSize );
+        const size_t index =
+            int(xColor[gid]) + int(yColor[gid] * 256) + int(zColor[gid] * 65536);


explain why it's like that

jafyvilla · 2016-10-04T13:10:24Z

brayns/io/NESTLoader.cpp

+    for( size_t i = 0; i < _nbElements; ++i )
+    {
+        const float time = _values[ i ];
+        // whith the next check, simulation only plays forward


I did it like this so the loading could be faster, considering only timestamps bigger than current. But now you can only play the simulation forward. If that's ok it should be documented.

Let's go for the timestamp, that makes everything more consistent, externally and internally. Modifications done.

Actually, I mean the if-clause at line 235. I added the if to spend less time reading the file, which I found slow. And if you keep it, it will only play the simulation nicely in one direction (forward).

Ok, I see what you mean. The only way to play the simulation backwards is to store the actual value of the spike intensity in the cache file itself, instead of computing it in the shader. Problem in doing this is that the speed of the decay will be stored in the cache file and cannot be changed at runtime. AFAIK, the simulation is always played forward. I suggest we keep the current implementation and document it. Ok with you?

Sure, I don't see the need of playing backwards for now.

jafyvilla · 2016-10-04T13:11:12Z

brayns/io/NESTLoader.cpp

+
+        // we store the frame on which the spike happens, as the renderer keeps
+        // track of the current frame (not timestamp)
+        _spikingTimes[ gid ] = time / NEST_TIMESTEP;


Times or frames, up to you (I think I prefer frames internally). But it should be consistent.

jafyvilla · 2016-10-04T13:11:39Z

brayns/io/NESTLoader.h

+     * @param filename File containing the circuit
+     * @param scene Scene in which spheres should be added
+     */
+    void importCircuit( const std::string& filename, Scene& scene, size_t& nbMaterials );


doxygen nbMaterials

jafyvilla · 2016-10-04T13:11:53Z

brayns/io/NESTLoader.h

+     * @param filename File containing the report
+     * @param scene Scene to which the simulation should be attached
+     */
+    bool importSpikeReport( const std::string& filename, Scene& scene );


doxygen return

jafyvilla · 2016-10-04T13:13:37Z

plugins/extensions/plugins/ZeroEQPlugin.cpp

@@ -334,6 +367,30 @@ bool ZeroEQPlugin::_requestFrameBuffers()
    return true;
 }

+bool ZeroEQPlugin::_requestSpikes()
+{
+    BRAYNS_INFO << "Spikes requested" << std::endl;


I wasn't aware this was already in place. Was it tested?

Not really for now. We need a data provider service that does not yet exist. Experimental for now.

favreau · 2016-10-05T14:43:26Z

Retest this please

Conflicts: brayns/io/MorphologyLoader.cpp tests/brayns.cpp

favreau assigned jafyvilla Sep 21, 2016

jafyvilla reviewed Sep 21, 2016

View reviewed changes

favreau mentioned this pull request Sep 21, 2016

Add support for NEST circuits (and spikes visualization) #29

Closed

jafyvilla reviewed Oct 4, 2016

View reviewed changes

favreau force-pushed the spikes branch from ae3f584 to 63e3c25 Compare October 5, 2016 14:37

favreau force-pushed the spikes branch 2 times, most recently from 5c190ad to 3f54e65 Compare October 5, 2016 14:51

Add support for NEST circuits (and spikes visualization)

91e5cbf

favreau force-pushed the spikes branch from 3f54e65 to 91e5cbf Compare October 6, 2016 07:42

Merge branch 'master' into spikes

9e507da

Conflicts: brayns/io/MorphologyLoader.cpp tests/brayns.cpp

jafyvilla merged commit 8e7bad2 into master Oct 6, 2016

favreau deleted the spikes branch October 6, 2016 13:22

		@@ -731,35 +730,38 @@ void OSPRayScene::commitSimulationData()
		const uint64_t frame = _sceneParameters.getTimestamp();

Add support for NEST circuits (and spikes visualization) #31

Add support for NEST circuits (and spikes visualization) #31

Conversation

favreau commented Sep 21, 2016

jafyvilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafyvilla Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafyvilla Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

favreau commented Oct 5, 2016

jafyvilla Sep 26, 2016 •

edited

Loading

jafyvilla Sep 26, 2016 •

edited

Loading