From d9cbd497172780c960254f11f4e62c584445ab90 Mon Sep 17 00:00:00 2001 From: Juan Hernando Vieites Date: Mon, 5 Dec 2016 10:49:09 +0100 Subject: [PATCH 1/3] Fix element order in Python wrapping of circuit accessors. The new implementation preserve the iteration order of the python object used as input GID set. The functions that were previously returning Python sets for GID sets now return sorted numpy arrays. It should still bt easy to perform set operations using numpy: https://docs.scipy.org/doc/numpy/reference/routines.set.html --- brain/circuit.cpp | 4 +- brain/circuit.h | 28 ++++++--- brain/python/CMakeLists.txt | 1 + brain/python/arrayHelpers.cpp | 61 +++++++++++++++++++- brain/python/arrayHelpers.h | 19 ++++++- brain/python/circuit.cpp | 98 ++++++++++++++++++++++++-------- brain/python/helpers.cpp | 113 +++++++++++++++++++++++++++++++++++++ brain/python/helpers.h | 47 ++++++++------- brain/python/neuron/morphology.cpp | 3 +- brion/circuit.cpp | 2 +- doc/Changelog.md | 5 ++ tests/brain/circuit.cpp | 8 +-- tests/brain/python/circuit.py | 40 ++++++++++++- 13 files changed, 364 insertions(+), 65 deletions(-) create mode 100644 brain/python/helpers.cpp diff --git a/brain/circuit.cpp b/brain/circuit.cpp index 7cfcbd0..75f3a14 100644 --- a/brain/circuit.cpp +++ b/brain/circuit.cpp @@ -161,7 +161,7 @@ size_ts Circuit::getMorphologyTypes( const GIDSet& gids ) const return _impl->getMTypes( gids ); } -Strings Circuit::getMorphologyNames() const +Strings Circuit::getMorphologyTypeNames() const { return _impl->getMorphologyNames(); } @@ -171,7 +171,7 @@ size_ts Circuit::getElectrophysiologyTypes( const GIDSet& gids ) const return _impl->getETypes( gids ); } -Strings Circuit::getElectrophysiologyNames() const +Strings Circuit::getElectrophysiologyTypeNames() const { return _impl->getElectrophysiologyNames(); } diff --git a/brain/circuit.h b/brain/circuit.h index 4430b90..dc4aa56 100644 --- a/brain/circuit.h +++ b/brain/circuit.h @@ -62,12 +62,15 @@ class Circuit BRAIN_API ~Circuit(); /** - * @return The set of GIDs for the given target name. + * @return The \if pybind array \else set \endif of GIDs for the given + * target name. * @throw std::runtime_error if the target cannot be found. */ BRAIN_API GIDSet getGIDs( const std::string& target ) const; - /** @return The set of all GIDs held by the circuit */ + /** @return The \if pybind array \else set \endif of all GIDs held by the + * circuit. + */ BRAIN_API GIDSet getGIDs() const; /** @@ -98,34 +101,41 @@ class Circuit BRAIN_API neuron::Morphologies loadMorphologies( const GIDSet& gids, Coordinates coords ) const; - /** @return The positions of the given cells. */ + /** @return The positions of the given cells in the iteration order of the + * input gids. + */ BRAIN_API Vector3fs getPositions( const GIDSet& gids ) const; - /** @return The morphology type indices of the given cells. */ + /** @return The morphology type indices of the given cells in the iteration + * order of the input gids. + */ BRAIN_API size_ts getMorphologyTypes( const GIDSet& gids ) const; /** * @return The morphology type names of the circuit, indexed by * getMorphologyTypes(). */ - BRAIN_API Strings getMorphologyNames() const; + BRAIN_API Strings getMorphologyTypeNames() const; - /** @return The electrophysiology type indices of the given cells. */ + /** @return The electrophysiology type indices of the given cells in the + * iteration order of the input gids. + */ BRAIN_API size_ts getElectrophysiologyTypes( const GIDSet& gids ) const; /** * @return The electrophysiology type names of the circuit, indexed by * getElectrophysiologyTypes(). */ - BRAIN_API Strings getElectrophysiologyNames() const; + BRAIN_API Strings getElectrophysiologyTypeNames() const; /** @return \if pybind A Nx4 numpy array with the \else The \endif - * local to world transformations of the given cells. + * local to world transformations of the given cells in the + * iteration */ BRAIN_API Matrix4fs getTransforms( const GIDSet& gids ) const; /** @return \if pybind A Nx4 numpy array with the \else The \endif - * local to world rotation of the given cells. + * local to world rotation of the given cells. */ BRAIN_API Quaternionfs getRotations( const GIDSet& gids ) const; diff --git a/brain/python/CMakeLists.txt b/brain/python/CMakeLists.txt index d247d23..1516947 100644 --- a/brain/python/CMakeLists.txt +++ b/brain/python/CMakeLists.txt @@ -27,6 +27,7 @@ docstrings(BRAIN_PYTHON_SOURCES BRAIN_PUBLIC_HEADERS list(APPEND BRAIN_PYTHON_SOURCES arrayHelpers.cpp + helpers.cpp spikeReportWriter.cpp spikeReportReader.cpp spikes.cpp diff --git a/brain/python/arrayHelpers.cpp b/brain/python/arrayHelpers.cpp index 3149a81..4be9817 100644 --- a/brain/python/arrayHelpers.cpp +++ b/brain/python/arrayHelpers.cpp @@ -176,6 +176,11 @@ void importArray() bp::class_< AbstractCustodian, AbstractCustodianPtr >( "_Custodian" ); } +bool isArray( const bp::object& o ) +{ + return PyArray_Check( o.ptr( )); +} + bp::object toNumpy( const brain::Matrix4f& matrix ) { npy_intp dims[2] = { 4, 4 }; @@ -195,6 +200,32 @@ bp::object toNumpy( const brain::Matrix4f& matrix ) namespace { + +template< typename T > +bool _copyGIDs( PyArrayObject* array, uint32_ts& result ) +{ + size_t size = PyArray_DIMS(array)[0]; + bool sorted = true; + + result.clear(); + result.reserve( size ); + + T last = 0; + for( size_t i = 0; i != size; ++i ) + { + const T gid = *static_cast< T* >(PyArray_GETPTR1( array, i )); + if( gid < 0 || ssize_t( gid ) > + ssize_t( std::numeric_limits< uint32_t >::max( ))) + PyErr_SetString( PyExc_ValueError, "Invalid input GID" ); + if( last >= gid ) + sorted = false; + else + sorted = gid; + result.push_back( gid ); + } + return sorted; +} + template< typename T > void _copyArrayToMatrix( PyArrayObject* array, Matrix4f& matrix ) { @@ -205,9 +236,37 @@ void _copyArrayToMatrix( PyArrayObject* array, Matrix4f& matrix ) } +bool gidsFromNumpy( const boost::python::object& object, uint32_ts& result ) +{ + PyArrayObject* array = reinterpret_cast< PyArrayObject* >( object.ptr( )); + if( PyArray_NDIM( array ) != 1 ) + { + PyErr_SetString( PyExc_ValueError, + "Cannot convert argument to GID set" ); + boost::python::throw_error_already_set(); + } + + switch( PyArray_TYPE( array )) + { + case NPY_LONG: return _copyGIDs< long >( array, result ); + case NPY_INT: return _copyGIDs< int >( array, result ); + case NPY_UINT: return _copyGIDs< unsigned int >( array, result ); + default:; + } + std::stringstream msg; + PyArray_Descr* desc = PyArray_DESCR( array ); + msg << "Cannot convert numpy array of type " << desc->kind + << desc->elsize << " to GID set" << std::endl; + PyErr_SetString( PyExc_ValueError, msg.str().c_str( )); + boost::python::throw_error_already_set(); + return false; // Unreachable +} + + +template <> Matrix4f fromNumpy( const bp::object& o ) { - if( !PyArray_Check( o.ptr( ))) + if( !isArray( o )) { PyErr_SetString( PyExc_ValueError, "Cannot convert object to Matrix4f" ); bp::throw_error_already_set(); diff --git a/brain/python/arrayHelpers.h b/brain/python/arrayHelpers.h index 1786f18..e635178 100644 --- a/brain/python/arrayHelpers.h +++ b/brain/python/arrayHelpers.h @@ -30,24 +30,37 @@ namespace brain void importArray(); +/** @return True is the python object is a numpy.ndarray, false otherwise. */ +bool isArray( const boost::python::object& object ); + template< typename T > boost::python::object toNumpy( std::vector< T >&& vector ); /** The custodian object must be copy constructible. In general it's expected - to be a std::shared_ptr or boost::shared_ptr. */ + * to be a std::shared_ptr or boost::shared_ptr. + */ template< typename T, typename U > boost::python::object toNumpy( const std::vector< T >& vector, const U& custodian ); /** The custodian object must be copy constructible. In general it's expected - to be a std::shared_ptr or boost::shared_ptr. */ + * to be a std::shared_ptr or boost::shared_ptr. + */ template< typename T, typename U > boost::python::object toNumpy( const T* array, size_t size, const U& custodian ); boost::python::object toNumpy( const Matrix4f& matrix ); -Matrix4f fromNumpy( const boost::python::object& object ); +/** Copy the GIDs from a numpy array into a uint32_t vector. + * @param object The python object representing the numpy array. + * @param result The destination array. + * @return true if the GIDs are in ascending order + */ +bool gidsFromNumpy( const boost::python::object& object, uint32_ts& result ); + +template +T fromNumpy( const boost::python::object& object ); } diff --git a/brain/python/circuit.cpp b/brain/python/circuit.cpp index 828e603..e3f6397 100644 --- a/brain/python/circuit.cpp +++ b/brain/python/circuit.cpp @@ -36,6 +36,45 @@ namespace brain namespace { +/* Reorders a collection with a random access iterator in place provided a pair + of iterators with the desired order. The order collection is overwritten. + http://stackoverflow.com/questions/838384 +*/ +template< typename T > +void _reorderDestructive( std::vector< T >& values, uint32_ts& order ) +{ + size_t remaining = order.size(); + for( uint32_t in_pos = 0; remaining > 0; ++in_pos ) + { + uint32_t out_pos = order[in_pos]; + if( out_pos == std::numeric_limits< uint32_t >::max( )) + continue; + --remaining; + T tmp = values[in_pos]; + for( uint32_t i; out_pos != in_pos; out_pos = i, --remaining ) + { + assert( out_pos != std::numeric_limits< uint32_t >::max( )); + std::swap( tmp, values[out_pos] ); + std::swap( order[out_pos], + i = std::numeric_limits< uint32_t >::max( )); + } + values[in_pos] = tmp; + } +} + +template< typename MemberFunction > +bp::object _getProperty( const Circuit& circuit, const MemberFunction& property, + bp::object cellSet ) +{ + uint32_ts mapping; + GIDSet gids; + gidsFromPython( cellSet, gids, mapping ); + auto values = (circuit.*property)( gids ); + if( !mapping.empty( )) + _reorderDestructive( values, mapping ); + return toNumpy( std::move( values )); +} + CircuitPtr Circuit_initFromURI( const std::string& uri ) { return CircuitPtr( new Circuit( URI( uri ))); @@ -43,37 +82,45 @@ CircuitPtr Circuit_initFromURI( const std::string& uri ) bp::object Circuit_getAllGIDs( const Circuit& circuit ) { - return toPythonSet( circuit.getGIDs( )); + return toNumpy( toVector( circuit.getGIDs( ))); } bp::object Circuit_getGIDs( const Circuit& circuit, const std::string& target ) { - return toPythonSet( circuit.getGIDs( target )); + return toNumpy( toVector( circuit.getGIDs( target ))); } bp::object Circuit_getRandomTargetGIDs( const Circuit& circuit, const float fraction, const std::string& target ) { - return toPythonSet( circuit.getRandomGIDs( fraction, target )); + return toNumpy( toVector( circuit.getRandomGIDs( fraction, target ))); } bp::object Circuit_getRandomGIDs( const Circuit& circuit, const float fraction ) { - return toPythonSet( circuit.getRandomGIDs( fraction )); + return toNumpy( toVector( circuit.getRandomGIDs( fraction ))); } #define GET_CIRCUIT_PROPERTY_FOR_GIDS(property) \ bp::object Circuit_get##property( const Circuit& circuit, bp::object gids ) \ { \ - return toNumpy( circuit.get##property( gidsFromPython( gids ))); \ + return _getProperty( circuit, &Circuit::get##property, gids ); \ } -GET_CIRCUIT_PROPERTY_FOR_GIDS(MorphologyTypes) -GET_CIRCUIT_PROPERTY_FOR_GIDS(ElectrophysiologyTypes) +GET_CIRCUIT_PROPERTY_FOR_GIDS( MorphologyTypes ) +GET_CIRCUIT_PROPERTY_FOR_GIDS( ElectrophysiologyTypes ) -bp::object Circuit_getMorphologyURIs( const Circuit& circuit, bp::object gids ) +bp::object Circuit_getMorphologyURIs( const Circuit& circuit, bp::object object ) { - return toPythonList( circuit.getMorphologyURIs( gidsFromPython( gids ))); + // Not trying to use a more general version of _getProperty because of its + // complexity. + uint32_ts mapping; + GIDSet gids; + gidsFromPython( object, gids, mapping ); + auto uris = circuit.getMorphologyURIs( gids ); + if( !mapping.empty( )) + _reorderDestructive( uris, mapping ); + return toPythonList( uris ); } #define GET_CIRCUIT_PROPERTY_VALUES(property) \ @@ -81,25 +128,32 @@ bp::object Circuit_getMorphologyURIs( const Circuit& circuit, bp::object gids ) { \ return toPythonList( circuit.get##property( )); \ } -GET_CIRCUIT_PROPERTY_VALUES(MorphologyNames) -GET_CIRCUIT_PROPERTY_VALUES(ElectrophysiologyNames) +GET_CIRCUIT_PROPERTY_VALUES(MorphologyTypeNames) +GET_CIRCUIT_PROPERTY_VALUES(ElectrophysiologyTypeNames) -bp::list Circuit_loadMorphologies( const Circuit& circuit, - bp::object gids, Circuit::Coordinates coords ) +bp::list Circuit_loadMorphologies( const Circuit& circuit, bp::object object, + Circuit::Coordinates coords ) { - return toPythonList( - circuit.loadMorphologies( gidsFromPython( gids ), coords )); + // Not trying to use a more general version of _getProperty because of its + // complexity. + uint32_ts mapping; + GIDSet gids; + gidsFromPython( object, gids, mapping ); + auto morphologies = circuit.loadMorphologies( gids , coords ); + if( !mapping.empty( )) + _reorderDestructive( morphologies, mapping ); + return toPythonList( morphologies ); } bp::object Circuit_getPositions( const Circuit& circuit, bp::object gids ) { - return toNumpy( circuit.getPositions( gidsFromPython( gids ))); + return _getProperty( circuit, &Circuit::getPositions, gids ); } bp::object Circuit_getTransforms( const Circuit& circuit, bp::object gids ) { bp::object matrices = - toNumpy( circuit.getTransforms( gidsFromPython( gids ))); + _getProperty( circuit, &Circuit::getTransforms, gids ); // We want the result to be indexed using regular mathematical notation // even if the actual storage is column-major. return matrices.attr( "transpose" )( 0, 2, 1 ); @@ -107,7 +161,7 @@ bp::object Circuit_getTransforms( const Circuit& circuit, bp::object gids ) bp::object Circuit_getRotations( const Circuit& circuit, bp::object gids ) { - return toNumpy( circuit.getRotations( gidsFromPython( gids ))); + return _getProperty( circuit, &Circuit::getRotations, gids ); } SynapsesWrapper Circuit_getAfferentSynapses( @@ -181,14 +235,14 @@ circuitWrapper .def( "morphology_types", Circuit_getMorphologyTypes, ( selfarg, bp::arg( "gids" )), DOXY_FN( brain::Circuit::getMorphologyTypes )) - .def( "morphology_names", Circuit_getMorphologyNames, ( selfarg ), - DOXY_FN( brain::Circuit::getMorphologyNames )) + .def( "morphology_type_names", Circuit_getMorphologyTypeNames, ( selfarg ), + DOXY_FN( brain::Circuit::getMorphologyTypeNames )) .def( "electrophysiology_types", Circuit_getElectrophysiologyTypes, ( selfarg, bp::arg( "gids" )), DOXY_FN( brain::Circuit::getElectrophysiologyTypes )) - .def( "electrophysiology_names", Circuit_getElectrophysiologyNames, + .def( "electrophysiology_type_names", Circuit_getElectrophysiologyTypeNames, ( selfarg), - DOXY_FN( brain::Circuit::getElectrophysiologyNames )) + DOXY_FN( brain::Circuit::getElectrophysiologyTypeNames )) .def( "transforms", Circuit_getTransforms, ( selfarg, bp::arg( "gids" )), DOXY_FN( brain::Circuit::getTransforms )) diff --git a/brain/python/helpers.cpp b/brain/python/helpers.cpp new file mode 100644 index 0000000..c09f4e4 --- /dev/null +++ b/brain/python/helpers.cpp @@ -0,0 +1,113 @@ +/* Copyright (c) 2013-2016, EPFL/Blue Brain Project + * Juan Hernando + * + * This file is part of Brion + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License version 3.0 as published + * by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "helpers.h" +#include "arrayHelpers.h" + +namespace brain +{ + +namespace +{ + +bool _gidsFromIterable( const boost::python::object& iterable, + uint32_ts& result ) +{ + bool sorted = true; + try + { + result.clear(); + result.reserve( len( iterable )); + // Copying the elements in the iterable to std vector for using insertion + // from an iterable + boost::python::stl_input_iterator< unsigned int > i( iterable ), end; + uint32_t last = 0; + for( ; i != end; ++i ) + { + const uint32_t gid = *i; + if( last >= gid ) + sorted = false; + else + last = gid; + result.push_back( gid ); + } + } + catch(...) + { + PyErr_SetString( PyExc_ValueError, + "Cannot convert argument to GID set" ); + boost::python::throw_error_already_set(); + } + return sorted; +} + +} + +GIDSet gidsFromPython( const boost::python::object& object ) +{ + uint32_ts vector; + + if( isArray( object )) + gidsFromNumpy( object, vector ); + else + _gidsFromIterable( object, vector ); + + GIDSet gids; + gids.insert( vector.begin(), vector.end( )); + return gids; +} + +void gidsFromPython( const boost::python::object& object, + GIDSet& result, uint32_ts& mapping ) +{ + uint32_ts vector; + + const bool sorted = + isArray( object ) ? gidsFromNumpy( object, vector ) : + _gidsFromIterable( object, vector ); + + std::unordered_map< uint32_t, uint32_t > gidToInput; + if( !sorted ) + { + gidToInput.reserve( vector.size( )); + // Building the GID to input index table + for( size_t i = 0; i != vector.size(); ++i ) + { + auto iter = gidToInput.insert( std::make_pair( vector[i], i )); + if( !iter.second ) + { + PyErr_SetString( PyExc_ValueError, "Repeated GID found" ); + boost::python::throw_error_already_set(); + } + } + } + + result.clear(); + result.insert( vector.begin(), vector.end( )); + + mapping.clear(); + if( !sorted ) + { + mapping.reserve( result.size( )); + for( auto gid : result ) + mapping.push_back( gidToInput[gid] ); + } +} + +} diff --git a/brain/python/helpers.h b/brain/python/helpers.h index 21bbc44..733f667 100644 --- a/brain/python/helpers.h +++ b/brain/python/helpers.h @@ -25,6 +25,8 @@ #include +#include + namespace brain { @@ -57,6 +59,15 @@ inline boost::python::list toPythonList( const std::vector< T >& vector ) return result; } +inline uint32_ts toVector( const GIDSet& gids ) +{ + uint32_ts result; + result.reserve( gids.size( )); + for( uint32_t gid : gids ) + result.push_back( gid ); + return result; +} + inline boost::python::object toPythonSet( const GIDSet& ids ) { boost::python::object set( boost::python::handle<>( PySet_New( 0 ))); @@ -70,26 +81,22 @@ inline boost::python::object toPythonSet( const GIDSet& ids ) return set; } -inline GIDSet gidsFromPython( const boost::python::object iterable ) -{ - GIDSet result; - try - { - boost::python::stl_input_iterator< unsigned int > i( iterable ), end; - uint32_ts vector; - vector.reserve( len( iterable )); - for( ; i != end; ++i) - vector.push_back( *i ); - result.insert( vector.begin(), vector.end( )); - } - catch(...) - { - PyErr_SetString( PyExc_ValueError, - "Cannot convert argument to GID set" ); - boost::python::throw_error_already_set(); - } - return result; -} +/** Copy the contents of a Python iterable into or numpy array into a GIDSet. */ +GIDSet gidsFromPython( const boost::python::object& object ); + +/** Copy the contents of a Python iterable or numpy array into a GIDSet and + return the correspondance map between elements in the iterable and the set. + @param object A Python iterable object or numpy.ndarray + @param result The out GID set. + @param result mapping If the input iterable is not sorted, this vector + contains at each position the position in the input iterable of the + elements as iterated in the result set. If the input iterable is + sorted this vector is empty. + This can be used to shuffle a vector sorted by the GIDSet to match + the iteration order of the Python iterable. +*/ +void gidsFromPython( const boost::python::object& object, + GIDSet& result, uint32_ts& mapping ); } #endif diff --git a/brain/python/neuron/morphology.cpp b/brain/python/neuron/morphology.cpp index 6fb619a..3dcc90d 100644 --- a/brain/python/neuron/morphology.cpp +++ b/brain/python/neuron/morphology.cpp @@ -103,7 +103,8 @@ MorphologyPtr Morphology_initFromURI( const std::string& uri ) MorphologyPtr Morphology_initFromURIAndTransform( const std::string& uri, bp::object transform ) { - return MorphologyPtr( new Morphology( URI( uri ), fromNumpy( transform ))); + return MorphologyPtr( new Morphology( URI( uri ), + fromNumpy< Matrix4f >( transform ))); } #define GET_MORPHOLOGY_ARRAY( Array ) \ diff --git a/brion/circuit.cpp b/brion/circuit.cpp index 5e16b53..bed60b6 100644 --- a/brion/circuit.cpp +++ b/brion/circuit.cpp @@ -103,7 +103,7 @@ class Circuit::Impl indices.reserve( gids.size( )); BOOST_FOREACH( const uint32_t gid, gids ) { - if ( gid > neurons.size( )) + if( gid > neurons.size( ) || gid == 0 ) { std::stringstream msg; msg << "Cell GID out of range: " << gid; diff --git a/doc/Changelog.md b/doc/Changelog.md index fa6d03c..da4a8f8 100644 --- a/doc/Changelog.md +++ b/doc/Changelog.md @@ -3,6 +3,11 @@ Changelog {#Changelog} # git master +* [115](https://github.com/BlueBrain/Brion/pull/113): + Changes and fixes in brian Python module: + - Functions that take gids now preserve the iteration order of the input in + the output arrays/lists. + - Replaced Python sets with numpy arrays in functions returning GID sets. * [113](https://github.com/BlueBrain/Brion/pull/113): Support for old circuits containing only synapse center positions. * [110](https://github.com/BlueBrain/Brion/pull/110): diff --git a/tests/brain/circuit.cpp b/tests/brain/circuit.cpp index c0b872e..1dc08ac 100644 --- a/tests/brain/circuit.cpp +++ b/tests/brain/circuit.cpp @@ -393,14 +393,14 @@ BOOST_AUTO_TEST_CASE(compare_mvd2_mvd3) const brain::size_ts& mtypes2 = circuit2.getMorphologyTypes( gids ); const brain::size_ts& etypes2 = circuit2.getElectrophysiologyTypes( gids ); - const brain::Strings& allMTypes2 = circuit2.getMorphologyNames(); - const brain::Strings& allETypes2 = circuit2.getElectrophysiologyNames(); + const brain::Strings& allMTypes2 = circuit2.getMorphologyTypeNames(); + const brain::Strings& allETypes2 = circuit2.getElectrophysiologyTypeNames(); const brain::URIs& names2 = circuit2.getMorphologyURIs( gids ); const brain::size_ts& mtypes3 = circuit3.getMorphologyTypes( gids ); const brain::size_ts& etypes3 = circuit3.getElectrophysiologyTypes( gids ); - const brain::Strings& allMTypes3 = circuit3.getMorphologyNames(); - const brain::Strings& allETypes3 = circuit3.getElectrophysiologyNames(); + const brain::Strings& allMTypes3 = circuit3.getMorphologyTypeNames(); + const brain::Strings& allETypes3 = circuit3.getElectrophysiologyTypeNames(); const brain::URIs& names3 = circuit3.getMorphologyURIs( gids ); BOOST_CHECK_EQUAL_COLLECTIONS( mtypes2.begin(), mtypes2.end(), diff --git a/tests/brain/python/circuit.py b/tests/brain/python/circuit.py index 758bbe9..b24c97a 100644 --- a/tests/brain/python/circuit.py +++ b/tests/brain/python/circuit.py @@ -18,6 +18,7 @@ import setup import brain +import numpy import unittest @@ -45,6 +46,36 @@ def test_gids(self): assert(len(self.circuit.random_gids(0.1)) == 100) assert(len(self.circuit.random_gids(0.1, 'Column')) == 100) + def test_iteration_order(self): + + def get(functor, *args): + a = [] + a += list(functor(self.circuit, [100], *args)) + a += list(functor(self.circuit, [20, 40], *args)) + a += list(functor(self.circuit, [7], *args)) + a += list(functor(self.circuit, [87, 89, 88], *args)) + a += list(functor(self.circuit, {997, 123, 1}, *args)) + + b = list(functor(self.circuit, [100, 20, 40, 7, 87, 89, 88] + + list({997, 123, 1}), *args)) + return a, b + + a, b = get(brain.Circuit.morphology_types) + assert(a == b) + a, b = get(brain.Circuit.morphology_uris) + assert(a == b) + a, b = get(brain.Circuit.load_morphologies, + brain.Circuit.Coordinates.local) + assert(len(a) == len(b)) + for i, j in zip(a, b): + assert(numpy.all(i.points() == j.points())) + + def test_morphology_uris(self): + uris = self.circuit.morphology_uris([1, 100, 1000]) + assert['sm090227a1-2_idC.h5' in uris[0]] + assert['sm101103b1-2_INT_idC.h5' in uris[1]] + assert['tkb060509a2_ch3_mc_n_el_100x_1.h5' in uris[2]] + def test_geometry(self): gids = self.circuit.gids() transforms = self.circuit.transforms(gids) @@ -62,7 +93,7 @@ def test_load_morphology(self): def test_metypes(self): count = self.circuit.num_neurons() - mnames = self.circuit.morphology_names() + mnames = self.circuit.morphology_type_names() assert(len(mnames) == 9) for t in ['L1_SLAC', 'L23_PC', 'L23_MC', 'L4_PC', 'L4_MC', 'L5_TTPC1', 'L5_MC', 'L6_TPC_L1', 'L6_MC']: @@ -74,7 +105,7 @@ def test_metypes(self): for t in mtypes: assert(mnames[t] == 'L23_PC') - enames = self.circuit.electrophysiology_names() + enames = self.circuit.electrophysiology_type_names() assert(len(enames) == 2) for t in ['cACint', 'cADpyr']: assert(t in enames) @@ -83,7 +114,12 @@ def test_metypes(self): for t in etypes: assert(enames[t] == 'cADpyr') + def test_repeated_gid(self): + self.assertRaises(ValueError, + lambda: self.circuit.positions([1, 3, 1])) + def test_bad_gids(self): + self.assertRaises(RuntimeError, lambda: self.circuit.positions([0])) self.assertRaises(RuntimeError, lambda: self.circuit.load_morphologies( [100000], brain.Circuit.Coordinates.local)) From b9617e9f893df38b2fde345727b3a32016d3b73e Mon Sep 17 00:00:00 2001 From: Juan Hernando Vieites Date: Tue, 6 Dec 2016 15:31:22 +0100 Subject: [PATCH 2/3] CR #117 --- brain/python/arrayHelpers.cpp | 2 +- brain/python/arrayHelpers.h | 2 +- brion/circuit.cpp | 3 +-- doc/Changelog.md | 5 +++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/brain/python/arrayHelpers.cpp b/brain/python/arrayHelpers.cpp index 4be9817..0090d5a 100644 --- a/brain/python/arrayHelpers.cpp +++ b/brain/python/arrayHelpers.cpp @@ -220,7 +220,7 @@ bool _copyGIDs( PyArrayObject* array, uint32_ts& result ) if( last >= gid ) sorted = false; else - sorted = gid; + last = gid; result.push_back( gid ); } return sorted; diff --git a/brain/python/arrayHelpers.h b/brain/python/arrayHelpers.h index e635178..ff7259c 100644 --- a/brain/python/arrayHelpers.h +++ b/brain/python/arrayHelpers.h @@ -30,7 +30,7 @@ namespace brain void importArray(); -/** @return True is the python object is a numpy.ndarray, false otherwise. */ +/** @return True if the python object is a numpy.ndarray, false otherwise. */ bool isArray( const boost::python::object& object ); template< typename T > diff --git a/brion/circuit.cpp b/brion/circuit.cpp index bed60b6..911c4df 100644 --- a/brion/circuit.cpp +++ b/brion/circuit.cpp @@ -22,7 +22,6 @@ #include #include -#include #include #include #include @@ -101,7 +100,7 @@ class Circuit::Impl std::vector indices; indices.reserve( gids.size( )); - BOOST_FOREACH( const uint32_t gid, gids ) + for( const uint32_t gid : gids ) { if( gid > neurons.size( ) || gid == 0 ) { diff --git a/doc/Changelog.md b/doc/Changelog.md index da4a8f8..10a8707 100644 --- a/doc/Changelog.md +++ b/doc/Changelog.md @@ -3,11 +3,12 @@ Changelog {#Changelog} # git master -* [115](https://github.com/BlueBrain/Brion/pull/113): - Changes and fixes in brian Python module: +* [117](https://github.com/BlueBrain/Brion/pull/117): + Changes and fixes in Brain Python module: - Functions that take gids now preserve the iteration order of the input in the output arrays/lists. - Replaced Python sets with numpy arrays in functions returning GID sets. + - Circuit::getXYZNames functions renamed to Circuit::getXYZTypeNames. * [113](https://github.com/BlueBrain/Brion/pull/113): Support for old circuits containing only synapse center positions. * [110](https://github.com/BlueBrain/Brion/pull/110): From 45bfc4c77a4498dc398e4d8c617ff09f4d7c815b Mon Sep 17 00:00:00 2001 From: Juan Hernando Vieites Date: Tue, 6 Dec 2016 16:57:40 +0100 Subject: [PATCH 3/3] CR2 #117 --- brain/python/arrayHelpers.cpp | 5 ++++- brain/python/helpers.h | 28 ++++++++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/brain/python/arrayHelpers.cpp b/brain/python/arrayHelpers.cpp index 0090d5a..810ec4d 100644 --- a/brain/python/arrayHelpers.cpp +++ b/brain/python/arrayHelpers.cpp @@ -204,7 +204,7 @@ namespace template< typename T > bool _copyGIDs( PyArrayObject* array, uint32_ts& result ) { - size_t size = PyArray_DIMS(array)[0]; + const size_t size = PyArray_DIMS(array)[0]; bool sorted = true; result.clear(); @@ -216,7 +216,10 @@ bool _copyGIDs( PyArrayObject* array, uint32_ts& result ) const T gid = *static_cast< T* >(PyArray_GETPTR1( array, i )); if( gid < 0 || ssize_t( gid ) > ssize_t( std::numeric_limits< uint32_t >::max( ))) + { PyErr_SetString( PyExc_ValueError, "Invalid input GID" ); + boost::python::throw_error_already_set(); + } if( last >= gid ) sorted = false; else diff --git a/brain/python/helpers.h b/brain/python/helpers.h index 733f667..a2a2b16 100644 --- a/brain/python/helpers.h +++ b/brain/python/helpers.h @@ -61,11 +61,7 @@ inline boost::python::list toPythonList( const std::vector< T >& vector ) inline uint32_ts toVector( const GIDSet& gids ) { - uint32_ts result; - result.reserve( gids.size( )); - for( uint32_t gid : gids ) - result.push_back( gid ); - return result; + return uint32_ts( gids.begin(), gids.end( )); } inline boost::python::object toPythonSet( const GIDSet& ids ) @@ -81,20 +77,20 @@ inline boost::python::object toPythonSet( const GIDSet& ids ) return set; } -/** Copy the contents of a Python iterable into or numpy array into a GIDSet. */ +/** Copy the contents of a Python iterable or numpy array into a GIDSet. */ GIDSet gidsFromPython( const boost::python::object& object ); /** Copy the contents of a Python iterable or numpy array into a GIDSet and - return the correspondance map between elements in the iterable and the set. - @param object A Python iterable object or numpy.ndarray - @param result The out GID set. - @param result mapping If the input iterable is not sorted, this vector - contains at each position the position in the input iterable of the - elements as iterated in the result set. If the input iterable is - sorted this vector is empty. - This can be used to shuffle a vector sorted by the GIDSet to match - the iteration order of the Python iterable. -*/ + * return the correspondance map between elements in the iterable and the set. + * @param object A Python iterable object or numpy.ndarray + * @param result The out GID set. + * @param mapping If the input iterable is not sorted, this vector + * contains at each position the position in the input iterable of the + * elements as iterated in the result set. If the input iterable is + * sorted this vector is empty. + * This can be used to shuffle a vector sorted by the GIDSet to match + * the iteration order of the Python iterable. + */ void gidsFromPython( const boost::python::object& object, GIDSet& result, uint32_ts& mapping );