Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

Add LFP functor from separate repo #4

Merged
merged 1 commit into from Feb 12, 2016
Merged

Conversation

jafyvilla
Copy link
Contributor

Move all the voltage-specific logic to the FieldFunctor,
so CompartmentLoader can be used with other type of reports
(e.g. currents in LFP).
Add radius attribute in Event, used in the CompartmentLoader.

@@ -24,6 +24,16 @@ set(FIVOX_PUBLIC_HEADERS
vsdLoader.h
)

# Try cloning lfpFivox into the 'fivox' folder
set(FIVOXLFP_DIR ${PROJECT_SOURCE_DIR}/fivox/lfpFivox)
git_external(${FIVOXLFP_DIR} ssh://bbpcode.epfl.ch/viz/lfpFivox master OPTIONAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this? I hope it won't get stuck in a ssh password prompt?

@jafyvilla jafyvilla force-pushed the add_lfp branch 2 times, most recently from 4944f18 to af74e7a Compare February 2, 2016 16:49
@jafyvilla
Copy link
Contributor Author

Updated

@@ -22,6 +22,7 @@
#define FIVOX_FIELDFUNCTOR_H

#include <fivox/eventFunctor.h> // base class
#include <brion/brion.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

types.h should be enough?

@jafyvilla jafyvilla force-pushed the add_lfp branch 4 times, most recently from ae8f610 to 3bdecf4 Compare February 10, 2016 12:30
@jafyvilla
Copy link
Contributor Author

Updated

@@ -49,8 +49,6 @@ class CompartmentLoader::Impl
, _report( _config.getReportSource( params.getReport( )),
brion::MODE_READ, _target)
{
_report.updateMapping( _target );

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was never needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if the report target and _target don't match? Why does this function exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

So updateMapping can be protected!? If no, doxygen of updateMapping needs an explanation.

(not for this PR, but I'm wondering what the point is)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's called update, hence you can change the GIDs you want reportables for w/o throwing away the already opened report file. RTNeuron for instance can update the GIDs at runtime which are requested by the user or could be automatically if the visible neurons change for instance. Always creating a new compartment report object for that would be the other option.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eile
Copy link
Contributor

eile commented Feb 10, 2016

Seems to me the UNSET value needs to be set more. For example, in the spikeLoader unset is currently 0.

eile pushed a commit to eile/Brion that referenced this pull request Feb 11, 2016
tribal-tec added a commit to BlueBrain/Brion that referenced this pull request Feb 11, 2016
@jafyvilla
Copy link
Contributor Author

I missed some UNSET checks after the last rebase, I will put them back. Anyway, related to that, I found a couple of things that we need to discuss (easier in person).

@jafyvilla
Copy link
Contributor Author

Done

@jafyvilla
Copy link
Contributor Author

The result we get with the lfpFunctor at the moment is hard to validate though, at least with actual simulation data. I was discussing with @chevtche that we can probably add a validationLoader, that creates a simple set of events (e.g. a straight line) with easy-to-debug values, for cases like this.
If we do that, it will go in a separate commit.

Move all the voltage-specific logic to the FieldFunctor,
so CompartmentLoader can be used with other type of reports
(e.g. currents in LFP).
Add radius attribute in Event, used in the CompartmentLoader.
Adapt tests and default values to latest TestData (370ebee).

Change-Id: I2a4ae9b62bcb2a4cf825124fa801bd8b2efac67b
@jafyvilla
Copy link
Contributor Author

Updated

eile added a commit that referenced this pull request Feb 12, 2016
Add LFP functor from separate repo
@eile eile merged commit 9556b0d into BlueBrain:master Feb 12, 2016
@jafyvilla jafyvilla deleted the add_lfp branch March 9, 2016 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants