From 742e31e9c1088207f62ede23894136c82e968486 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:44:08 -0800 Subject: [PATCH 01/16] move electrodes table creation to separate function --- src/nwb/NWBFile.cpp | 96 +++++++++++++++++++++++---------------------- src/nwb/NWBFile.hpp | 25 +++++++++--- 2 files changed, 68 insertions(+), 53 deletions(-) diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index b19dfb28..13c99448 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -28,7 +28,7 @@ constexpr SizeType CHUNK_XSIZE = constexpr SizeType SPIKE_CHUNK_XSIZE = 8; // TODO - replace with io settings input -std::vector NWBFile::emptyContainerIndexes = {}; +std::vector NWBFile::m_emptyContainerIndexes = {}; // Initialize the static registered_ member to trigger registration REGISTER_SUBCLASS_IMPL(NWBFile) @@ -172,6 +172,43 @@ Status NWBFile::createFileStructure(const std::string& identifierText, return Status::Success; } +Status NWBFile::createElectrodesTable( + std::vector recordingArrays +) +{ + std::unique_ptr electrodeTable = std::make_unique(m_io); + electrodeTable->initialize(); + for (const auto& channelVector : recordingArrays) { + electrodeTable->addElectrodes(channelVector); + } + + for (size_t i = 0; i < recordingArrays.size(); ++i) { + const auto& channelVector = recordingArrays[i]; + + // Setup electrodes and devices + std::string groupName = channelVector[0].getGroupName(); + std::string devicePath = AQNWB::mergePaths("/general/devices", groupName); + std::string electrodePath = + AQNWB::mergePaths("/general/extracellular_ephys", groupName); + + // Check if device exists for groupName, create device and electrode group + // if it does not + if (!m_io->objectExists(devicePath)) { + NWB::Device device = NWB::Device(devicePath, m_io); + device.initialize("description", "unknown"); + + NWB::ElectrodeGroup elecGroup = NWB::ElectrodeGroup(electrodePath, m_io); + elecGroup.initialize("description", "unknown", device); + } + } + + // write electrodes information to datasets + // (requires that ElectrodeGroup data is written) + electrodeTable->finalize(); + + return Status::Success; +} + Status NWBFile::createElectricalSeries( std::vector recordingArrays, std::vector recordingNames, @@ -191,58 +228,34 @@ Status NWBFile::createElectricalSeries( bool electrodeTableCreated = m_io->objectExists(ElectrodeTable::electrodeTablePath); if (!electrodeTableCreated) { - m_electrodeTable = std::make_unique(m_io); - m_electrodeTable->initialize(); - - // Add electrode information to table (does not write to datasets yet) - for (const auto& channelVector : recordingArrays) { - m_electrodeTable->addElectrodes(channelVector); - } + std::cerr << "NWBFile::createElectricalSeries requires an electrodes table to be present" << std::endl; + return Status::Failure; } // Create datasets for (size_t i = 0; i < recordingArrays.size(); ++i) { const auto& channelVector = recordingArrays[i]; const std::string& recordingName = recordingNames[i]; - - // Setup electrodes and devices - std::string groupName = channelVector[0].getGroupName(); - std::string devicePath = AQNWB::mergePaths("/general/devices", groupName); - std::string electrodePath = - AQNWB::mergePaths("/general/extracellular_ephys", groupName); std::string electricalSeriesPath = - AQNWB::mergePaths(acquisitionPath, recordingName); - - // Check if device exists for groupName, create device and electrode group - // if it does not - if (!m_io->objectExists(devicePath)) { - Device device = Device(devicePath, m_io); - device.initialize("description", "unknown"); - - ElectrodeGroup elecGroup = ElectrodeGroup(electrodePath, m_io); - elecGroup.initialize("description", "unknown", device); - } + AQNWB::mergePaths(m_acquisitionPath, recordingName); // Setup electrical series datasets auto electricalSeries = std::make_unique(electricalSeriesPath, m_io); - electricalSeries->initialize( + Status esStatus = electricalSeries->initialize( dataType, channelVector, "Stores continuously sampled voltage data from an " "extracellular ephys recording", SizeArray {0, channelVector.size()}, SizeArray {CHUNK_XSIZE, 0}); + if (esStatus != Status::Success) { + return esStatus; + } recordingContainers->addContainer(std::move(electricalSeries)); containerIndexes.push_back(recordingContainers->size() - 1); } - // write electrode information to datasets - // (requires that the ElectrodeGroup has been written) - if (!electrodeTableCreated) { - m_electrodeTable->finalize(); - } - return Status::Success; } @@ -265,13 +278,8 @@ Status NWBFile::createSpikeEventSeries( bool electrodeTableCreated = m_io->objectExists(ElectrodeTable::electrodeTablePath); if (!electrodeTableCreated) { - m_electrodeTable = std::make_unique(m_io); - m_electrodeTable->initialize(); - - // Add electrode information to table (does not write to datasets yet) - for (const auto& channelVector : recordingArrays) { - m_electrodeTable->addElectrodes(channelVector); - } + std::cerr << "NWBFile::createElectricalSeries requires an electrodes table to be present" << std::endl; + return Status::Failure; } // Create datasets @@ -285,7 +293,7 @@ Status NWBFile::createSpikeEventSeries( std::string electrodePath = AQNWB::mergePaths("/general/extracellular_ephys", groupName); std::string spikeEventSeriesPath = - AQNWB::mergePaths(acquisitionPath, recordingName); + AQNWB::mergePaths(m_acquisitionPath, recordingName); // Check if device exists for groupName, create device and electrode group // if not @@ -320,12 +328,6 @@ Status NWBFile::createSpikeEventSeries( containerIndexes.push_back(recordingContainers->size() - 1); } - // write electrode information to datasets - // (requires that the ElectrodeGroup has been written) - if (!electrodeTableCreated) { - m_electrodeTable->finalize(); - } - return Status::Success; } @@ -341,7 +343,7 @@ Status NWBFile::createAnnotationSeries(std::vector recordingNames, const std::string& recordingName = recordingNames[i]; std::string annotationSeriesPath = - AQNWB::mergePaths(acquisitionPath, recordingName); + AQNWB::mergePaths(m_acquisitionPath, recordingName); // Setup annotation series datasets auto annotationSeries = diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 93e81b26..27b7af2f 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -92,6 +92,20 @@ class NWBFile : public Container */ Status finalize(); + /** + * @brief Create ElectrodesTable. + * Note, this function will fail if the file is in a mode where + * new objects cannot be added, which can be checked via + * nwbfile.io->canModifyObjects() + * @param recordingArrays vector of ChannelVector indicating the electrodes to + * add to the table. This vector should contain all the + * electrodes that are detected by the acquisition system, + * not only those being actively recorded from. + * @return Status The status of the object creation operation. + */ + Status createElectrodesTable( + std::vector recordingArrays); + /** * @brief Create ElectricalSeries objects to record data into. * Created objects are stored in recordingContainers. @@ -114,7 +128,7 @@ class NWBFile : public Container std::vector recordingNames, const IO::BaseDataType& dataType = IO::BaseDataType::I16, RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = emptyContainerIndexes); + std::vector& containerIndexes = m_emptyContainerIndexes); /** * @brief Create SpikeEventSeries objects to record data into. @@ -135,7 +149,7 @@ class NWBFile : public Container std::vector recordingNames, const IO::BaseDataType& dataType = IO::BaseDataType::I16, RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = emptyContainerIndexes); + std::vector& containerIndexes = m_emptyContainerIndexes); /** @brief Create AnnotationSeries objects to record data into. * Created objects are stored in recordingContainers. @@ -149,7 +163,7 @@ class NWBFile : public Container Status createAnnotationSeries( std::vector recordingNames, RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = emptyContainerIndexes); + std::vector& containerIndexes = m_emptyContainerIndexes); DEFINE_REGISTERED_FIELD(readElectrodeTable, ElectrodeTable, @@ -246,10 +260,9 @@ class NWBFile : public Container const std::array, N>& specVariables); - inline const static std::string acquisitionPath = "/acquisition"; - static std::vector emptyContainerIndexes; + inline const static std::string m_acquisitionPath = "/acquisition"; + static std::vector m_emptyContainerIndexes; -private: /** * @brief The ElectrodeTable for the file */ From 6b4b62f5c27b1348c6b19a8054cf46f367b73508 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:44:38 -0800 Subject: [PATCH 02/16] update workflows for electrodes table creation --- tests/examples/testWorkflowExamples.cpp | 5 +++++ tests/examples/test_ecephys_data_read.cpp | 1 + tests/testRecordingWorkflow.cpp | 11 +++++++---- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/examples/testWorkflowExamples.cpp b/tests/examples/testWorkflowExamples.cpp index 4e1362ef..a3eb5a10 100644 --- a/tests/examples/testWorkflowExamples.cpp +++ b/tests/examples/testWorkflowExamples.cpp @@ -51,6 +51,11 @@ TEST_CASE("workflowExamples") REQUIRE(initStatus == Status::Success); // [example_workflow_nwbfile_snippet] + // [example_workflow_electrodes_table_snippet] + Status elecTableStatus =nwbfile->createElectrodesTable(mockRecordingArrays); + REQUIRE(elecTableStatus == Status::Success); + // [example_workflow_electrodes_table_snippet] + // [example_workflow_datasets_snippet] std::vector containerIndexes; Status elecSeriesStatus = diff --git a/tests/examples/test_ecephys_data_read.cpp b/tests/examples/test_ecephys_data_read.cpp index 0556e85c..867de663 100644 --- a/tests/examples/test_ecephys_data_read.cpp +++ b/tests/examples/test_ecephys_data_read.cpp @@ -67,6 +67,7 @@ TEST_CASE("ElectricalSeriesReadExample", "[ecephys]") std::make_unique(); // create a new ElectricalSeries + nwbfile.createElectrodesTable(mockArrays); Status resultCreate = nwbfile.createElectricalSeries( mockArrays, mockChannelNames, dataType, recordingContainers.get()); REQUIRE(resultCreate == Status::Success); diff --git a/tests/testRecordingWorkflow.cpp b/tests/testRecordingWorkflow.cpp index 62672104..aa581811 100644 --- a/tests/testRecordingWorkflow.cpp +++ b/tests/testRecordingWorkflow.cpp @@ -48,16 +48,19 @@ TEST_CASE("writeContinuousData", "[recording]") std::unique_ptr nwbfile = std::make_unique(io); nwbfile->initialize(generateUuid()); - // 4. create datasets and add to recording containers + // 4. create an electrodes table. + nwbfile->createElectrodesTable(mockRecordingArrays); + + // 5. create datasets and add to recording containers nwbfile->createElectricalSeries(mockRecordingArrays, mockChannelNames, BaseDataType::F32, recordingContainers.get()); - // 5. start the recording + // 6. start the recording io->startRecording(); - // 6. write data during the recording + // 7. write data during the recording bool isRecording = true; while (isRecording) { // write data to the file for each channel @@ -98,7 +101,7 @@ TEST_CASE("writeContinuousData", "[recording]") } } - // 7. stop the recording and finalize the file + // 8. stop the recording and finalize the file io->stopRecording(); nwbfile->finalize(); From 39ae5517cf7e73e028eb67b3ff2e6cc55a2ff4b5 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:44:52 -0800 Subject: [PATCH 03/16] add helper function for dataset size --- src/io/BaseIO.hpp | 9 +++++++++ src/io/hdf5/HDF5IO.cpp | 11 +++++++++++ src/io/hdf5/HDF5IO.hpp | 9 +++++++++ 3 files changed, 29 insertions(+) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index 7c506b4a..b7bd3349 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -441,6 +441,15 @@ class BaseIO virtual std::unique_ptr getDataSet( const std::string& path) = 0; + /** + * @brief Returns the size of the dataset for each dimension. + * @param path The location in the file of the dataset. + * @param ndims The number of dimensions in the dataset, defaults to 1. + * @return The dataset size. + */ + virtual std::vector getDatasetSize(const std::string path, + const size_t ndims = 1) = 0; + /** * @brief Convenience function for creating NWB related attributes. * @param path The location of the object in the file. diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index e523f3f4..ca25c28b 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -1106,6 +1106,17 @@ HDF5IO::getStorageObjects(const std::string& path, return objects; } +std::vector HDF5IO::getDatasetSize(const std::string path, + const size_t ndims) +{ + H5::DataSet dataset = m_file->openDataSet(path); + H5::DataSpace dataspace = dataset.getSpace(); + std::vector dims(ndims); + dataspace.getSimpleExtentDims(dims.data()); + + return std::vector(dims.begin(), dims.end()); +} + std::unique_ptr HDF5IO::getDataSet( const std::string& path) { diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index d89872fc..6947aa7d 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -268,6 +268,15 @@ class HDF5IO : public BaseIO std::unique_ptr getDataSet( const std::string& path) override; + /** + * @brief Returns the size of the dataset for each dimension. + * @param path The location in the file of the dataset. + * @param ndims The number of dimensions in the dataset, defaults to 1. + * @return The dataset size. + */ + std::vector getDatasetSize(const std::string path, + const size_t ndims = 1) override; + /** * @brief Checks whether a Dataset, Group, or Link already exists at the * location in the file. From 3128d22b050003d4a379301cb4e38befb4a37e5d Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:45:13 -0800 Subject: [PATCH 04/16] add status checks, warnings for invalid electrodes values --- src/nwb/ecephys/ElectricalSeries.cpp | 15 ++++++++++++++- src/nwb/ecephys/ElectricalSeries.hpp | 6 ++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/nwb/ecephys/ElectricalSeries.cpp b/src/nwb/ecephys/ElectricalSeries.cpp index 820d7b04..25a9dbcf 100644 --- a/src/nwb/ecephys/ElectricalSeries.cpp +++ b/src/nwb/ecephys/ElectricalSeries.cpp @@ -21,7 +21,7 @@ ElectricalSeries::ElectricalSeries(const std::string& path, ElectricalSeries::~ElectricalSeries() {} /** Initialization function*/ -void ElectricalSeries::initialize(const IO::BaseDataType& dataType, +Status ElectricalSeries::initialize(const IO::BaseDataType& dataType, const Types::ChannelVector& channelVector, const std::string& description, const SizeArray& dsetSize, @@ -42,10 +42,21 @@ void ElectricalSeries::initialize(const IO::BaseDataType& dataType, this->m_channelVector = channelVector; + // get the number of electrodes from the electrode table + std::string idPath = AQNWB::mergePaths(ElectrodeTable::electrodeTablePath, "id"); + std::vector elecTableDsetSize = m_io->getDatasetSize(idPath); + SizeType numElectrodes = elecTableDsetSize[0]; + // setup variables based on number of channels std::vector electrodeInds(channelVector.size()); std::vector channelConversions(channelVector.size()); for (size_t i = 0; i < channelVector.size(); ++i) { + SizeType globalIndex = channelVector[i].getGlobalIndex(); + if (globalIndex >= numElectrodes) { + std::cerr << "ElectricalSeries::initialize electrode index " << globalIndex + << " is out of range. Max index is " << (numElectrodes - 1) << std::endl; + return Status::Failure; + } electrodeInds[i] = static_cast(channelVector[i].getGlobalIndex()); channelConversions[i] = channelVector[i].getConversion(); } @@ -89,6 +100,8 @@ void ElectricalSeries::initialize(const IO::BaseDataType& dataType, m_io->createReferenceAttribute(ElectrodeTable::electrodeTablePath, AQNWB::mergePaths(getPath(), "electrodes"), "table"); + + return Status::Success; } Status ElectricalSeries::writeChannel(SizeType channelInd, diff --git a/src/nwb/ecephys/ElectricalSeries.hpp b/src/nwb/ecephys/ElectricalSeries.hpp index ca7d23fb..e2ea32af 100644 --- a/src/nwb/ecephys/ElectricalSeries.hpp +++ b/src/nwb/ecephys/ElectricalSeries.hpp @@ -49,9 +49,11 @@ class ElectricalSeries : public TimeSeries * @param resolution Smallest meaningful difference between values in data, * stored in the specified by unit * @param offset Scalar to add to the data after scaling by ‘conversion’ to - * finalize its coercion to the specified ‘unit' + * finalize its coercion to the specified ‘unit'\ + * @return The status of the initialization operation. + */ - void initialize(const IO::BaseDataType& dataType, + Status initialize(const IO::BaseDataType& dataType, const Types::ChannelVector& channelVector, const std::string& description, const SizeArray& dsetSize, From e6225e5e8726c2e4684ab0b3338029a39c8ee3d2 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:45:24 -0800 Subject: [PATCH 05/16] add electrodes table tests --- tests/testNWBFile.cpp | 124 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 122 insertions(+), 2 deletions(-) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index f87165f2..44c4f339 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -59,6 +59,119 @@ TEST_CASE("initialize", "[nwb]") io->close(); } +TEST_CASE("createElectrodesTable", "[nwb]") +{ + std::string filename = getTestFilePath("createElectrodesTable.nwb"); + + // initialize nwbfile object and create base structure + std::shared_ptr io = + std::make_shared(filename); + io->open(); + NWB::NWBFile nwbfile(io); + nwbfile.initialize(generateUuid()); + + // create the Electrodes Table + std::vector mockArrays = getMockChannelArrays(1, 2); + Status resultCreate = nwbfile.createElectrodesTable(mockArrays); + REQUIRE(resultCreate == Status::Success); +} + +TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") +{ + std::string filename = getTestFilePath("createElectricalSeriesWithSubset.nwb"); + + // initialize nwbfile object and create base structure + std::shared_ptr io = + std::make_shared(filename); + io->open(); + NWB::NWBFile nwbfile(io); + nwbfile.initialize(generateUuid()); + + // Create electrode table with full set of electrodes (4 channels) + std::vector allElectrodes = getMockChannelArrays(4, 1); + Status resultCreateTable = nwbfile.createElectrodesTable(allElectrodes); + REQUIRE(resultCreateTable == Status::Success); + + // Create electrical series with subset of electrodes (2 channels) + SizeType numChannels = 2; + std::vector recordingElectrodes = getMockChannelArrays(numChannels, 1); + std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + std::unique_ptr recordingContainers = + std::make_unique(); + Status resultCreateES = + nwbfile.createElectricalSeries(recordingElectrodes, + recordingNames, + BaseDataType::F32, + recordingContainers.get()); + REQUIRE(resultCreateES == Status::Success); + + // Write some test data to verify recording works + Status resultStart = io->startRecording(); + REQUIRE(resultStart == Status::Success); + + std::vector mockData = {1.0f, 2.0f, 3.0f}; + std::vector mockTimestamps = {0.1, 0.2, 0.3}; + std::vector positionOffset = {0, 0}; + std::vector dataShape = {mockData.size(), 0}; + + for (size_t i = 0; i < recordingElectrodes.size(); ++i) { + for (size_t j = 0; j < recordingElectrodes[i].size(); ++j) { + recordingContainers->writeElectricalSeriesData( + 0, recordingElectrodes[i][j], mockData.size(), mockData.data(), mockTimestamps.data()); + } + } + + nwbfile.finalize(); +} + +TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") +{ + std::string filename = getTestFilePath("createElectricalSeriesNoTable.nwb"); + + // initialize nwbfile object and create base structure + std::shared_ptr io = + std::make_shared(filename); + io->open(); + NWB::NWBFile nwbfile(io); + nwbfile.initialize(generateUuid()); + + // Attempt to create electrical series without creating electrodes table first + std::vector recordingElectrodes = getMockChannelArrays(1, 2); + std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + Status resultCreateES = nwbfile.createElectricalSeries( + recordingElectrodes, recordingNames, BaseDataType::F32); + REQUIRE(resultCreateES == Status::Failure); + + nwbfile.finalize(); +} + +TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") +{ + std::string filename = getTestFilePath("createElectricalSeriesOutOfRange.nwb"); + + // initialize nwbfile object and create base structure + std::shared_ptr io = + std::make_shared(filename); + io->open(); + NWB::NWBFile nwbfile(io); + nwbfile.initialize(generateUuid()); + + // Create electrode table with 2 channels + std::vector tableElectrodes = getMockChannelArrays(2, 1); + Status resultCreateTable = nwbfile.createElectrodesTable(tableElectrodes); + REQUIRE(resultCreateTable == Status::Success); + + // Attempt to create electrical series with channels having higher indices + // Create mock channels with global indices > 1 (out of range of table) + std::vector recordingElectrodes = getMockChannelArrays(4, 1); + + std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + Status resultCreateES = nwbfile.createElectricalSeries( + recordingElectrodes, recordingNames, BaseDataType::F32); + REQUIRE(resultCreateES == Status::Failure); +} + + TEST_CASE("createElectricalSeries", "[nwb]") { std::string filename = getTestFilePath("createElectricalSeries.nwb"); @@ -70,8 +183,11 @@ TEST_CASE("createElectricalSeries", "[nwb]") NWB::NWBFile nwbfile(io); nwbfile.initialize(generateUuid()); + // create the Electrodes Table + std::vector mockArrays = getMockChannelArrays(); + nwbfile.createElectrodesTable(mockArrays); + // create Electrical Series - std::vector mockArrays = getMockChannelArrays(1, 2); std::vector mockChannelNames = getMockChannelArrayNames("esdata"); std::unique_ptr recordingContainers = @@ -137,8 +253,11 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") NWB::NWBFile nwbfile(io); nwbfile.initialize(generateUuid()); - // create Electrical Series + // create ElectrodesTable std::vector mockArrays = getMockChannelArrays(2, 2); + nwbfile.createElectrodesTable(mockArrays); + + // create Electrical Series std::vector mockChannelNames = getMockChannelArrayNames("esdata"); std::unique_ptr recordingContainers = @@ -278,6 +397,7 @@ TEST_CASE("setCanModifyObjectsMode", "[nwb]") std::vector mockArrays = getMockChannelArrays(1, 2); std::vector mockChannelNames = getMockChannelArrayNames("esdata"); + nwbfile.createElectrodesTable(mockArrays); // create the Electrodes Table Status resultCreatePostStart = nwbfile.createElectricalSeries( mockArrays, mockChannelNames, BaseDataType::F32); REQUIRE(resultCreatePostStart == Status::Failure); From 830aea1b50c5d4213ae7d9379bdeb424b60f872a Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:45:32 -0800 Subject: [PATCH 06/16] update workflow documentation --- docs/pages/userdocs/workflow.dox | 33 ++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/docs/pages/userdocs/workflow.dox b/docs/pages/userdocs/workflow.dox index 155fc61c..4b9e980a 100644 --- a/docs/pages/userdocs/workflow.dox +++ b/docs/pages/userdocs/workflow.dox @@ -14,12 +14,14 @@ * used for managing \ref AQNWB::NWB::Container "Container" objects for storing recordings. * 3. Create the \ref AQNWB::NWB::NWBFile "NWBFile" object used for managing and creating NWB * file contents. - * 4. Create the \ref AQNWB::NWB::Container "Container" objects (e.g., + * 4. Create the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object used for saving + * electrodes information. + * 5. Create the \ref AQNWB::NWB::Container "Container" objects (e.g., * \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries") used for recording and add them * to the \ref AQNWB::NWB::RecordingContainers "RecordingContainers". - * 5. Start the recording. - * 6. Write data. - * 7. Stop the recording and close the \ref AQNWB::NWB::NWBFile "NWBFile". + * 6. Start the recording. + * 7. Write data. + * 8. Stop the recording and close the \ref AQNWB::NWB::NWBFile "NWBFile". * * Below, we walk through these steps in more detail. * @@ -52,7 +54,22 @@ * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_nwbfile_snippet * * - * \subsection create_datasets 4. Create datasets and add to RecordingContainers. + * \subsection create_nwbfile 4. Create the ElectrodesTable object. + * + * Next, constructs the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object, + * using the `recordingArrays` as an input. Note that in many cases, you will be recording from + * all electrodes detected by the device and this `recordingArrays` structure will be the same as + * is used to create the \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries". However in some cases + * (e.g. Neuropixels), you may be recording from a subset of all available electrodes. In this case, + * you should create the ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" with all electrodes + * detected by the acquisition system, not just those being actively recorded from. This approach is + * critical for mapping information about which \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries" + * were recorded from which electrodes. + * + * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_electrodes_table_snippet + * + * + * \subsection create_datasets 5. Create datasets and add to RecordingContainers. * * Next, create the different data types (e.g. \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries" * or other AQNWB::NWB::TimeSeries "TimeSeries") that you would like to write data into. After @@ -67,7 +84,7 @@ * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_datasets_snippet * * - * \subsection start_recording 5. Start the recording. + * \subsection start_recording 6. Start the recording. * * Then, start the recording process with a call to the ``startRecording`` function of the I/O object. * @@ -81,7 +98,7 @@ * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_start_snippet * * - * \subsection write_data 6. Write data. + * \subsection write_data 7. Write data. * * During the recording process, use the \ref AQNWB::NWB::RecordingContainers "RecordingContainers" * as an interface to access the various \ref AQNWB::NWB::Container "Container" object and corresponding @@ -91,7 +108,7 @@ * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_write_snippet * * - * \subsection stop_recording 7. Stop the recording and finalize the file. + * \subsection stop_recording 8. Stop the recording and finalize the file. * * When the recording process is finished, call `stopRecording` from the I/O object * to flush any data and close the file. From fe20dd230f680b5e80d05c3c577eb8e4ed0064b5 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:46:30 -0800 Subject: [PATCH 07/16] fix formatting --- src/io/BaseIO.hpp | 2 +- src/io/hdf5/HDF5IO.cpp | 2 +- src/io/hdf5/HDF5IO.hpp | 2 +- src/nwb/NWBFile.cpp | 16 ++++++---- src/nwb/NWBFile.hpp | 6 ++-- src/nwb/ecephys/ElectricalSeries.cpp | 22 +++++++------- src/nwb/ecephys/ElectricalSeries.hpp | 14 ++++----- tests/examples/testWorkflowExamples.cpp | 5 ++-- tests/testNWBFile.cpp | 39 ++++++++++++++++--------- 9 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index b7bd3349..bc6950e2 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -447,7 +447,7 @@ class BaseIO * @param ndims The number of dimensions in the dataset, defaults to 1. * @return The dataset size. */ - virtual std::vector getDatasetSize(const std::string path, + virtual std::vector getDatasetSize(const std::string path, const size_t ndims = 1) = 0; /** diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index ca25c28b..fa09aebf 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -1106,7 +1106,7 @@ HDF5IO::getStorageObjects(const std::string& path, return objects; } -std::vector HDF5IO::getDatasetSize(const std::string path, +std::vector HDF5IO::getDatasetSize(const std::string path, const size_t ndims) { H5::DataSet dataset = m_file->openDataSet(path); diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index 6947aa7d..8879b4a1 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -274,7 +274,7 @@ class HDF5IO : public BaseIO * @param ndims The number of dimensions in the dataset, defaults to 1. * @return The dataset size. */ - std::vector getDatasetSize(const std::string path, + std::vector getDatasetSize(const std::string path, const size_t ndims = 1) override; /** diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 13c99448..062b2943 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -173,10 +173,10 @@ Status NWBFile::createFileStructure(const std::string& identifierText, } Status NWBFile::createElectrodesTable( - std::vector recordingArrays -) + std::vector recordingArrays) { - std::unique_ptr electrodeTable = std::make_unique(m_io); + std::unique_ptr electrodeTable = + std::make_unique(m_io); electrodeTable->initialize(); for (const auto& channelVector : recordingArrays) { electrodeTable->addElectrodes(channelVector); @@ -205,7 +205,7 @@ Status NWBFile::createElectrodesTable( // write electrodes information to datasets // (requires that ElectrodeGroup data is written) electrodeTable->finalize(); - + return Status::Success; } @@ -228,7 +228,9 @@ Status NWBFile::createElectricalSeries( bool electrodeTableCreated = m_io->objectExists(ElectrodeTable::electrodeTablePath); if (!electrodeTableCreated) { - std::cerr << "NWBFile::createElectricalSeries requires an electrodes table to be present" << std::endl; + std::cerr << "NWBFile::createElectricalSeries requires an electrodes table " + "to be present" + << std::endl; return Status::Failure; } @@ -278,7 +280,9 @@ Status NWBFile::createSpikeEventSeries( bool electrodeTableCreated = m_io->objectExists(ElectrodeTable::electrodeTablePath); if (!electrodeTableCreated) { - std::cerr << "NWBFile::createElectricalSeries requires an electrodes table to be present" << std::endl; + std::cerr << "NWBFile::createElectricalSeries requires an electrodes table " + "to be present" + << std::endl; return Status::Failure; } diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 27b7af2f..c8feaf4c 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -98,9 +98,9 @@ class NWBFile : public Container * new objects cannot be added, which can be checked via * nwbfile.io->canModifyObjects() * @param recordingArrays vector of ChannelVector indicating the electrodes to - * add to the table. This vector should contain all the - * electrodes that are detected by the acquisition system, - * not only those being actively recorded from. + * add to the table. This vector should contain all the + * electrodes that are detected by the acquisition + * system, not only those being actively recorded from. * @return Status The status of the object creation operation. */ Status createElectrodesTable( diff --git a/src/nwb/ecephys/ElectricalSeries.cpp b/src/nwb/ecephys/ElectricalSeries.cpp index 25a9dbcf..5f0a9fe8 100644 --- a/src/nwb/ecephys/ElectricalSeries.cpp +++ b/src/nwb/ecephys/ElectricalSeries.cpp @@ -22,13 +22,13 @@ ElectricalSeries::~ElectricalSeries() {} /** Initialization function*/ Status ElectricalSeries::initialize(const IO::BaseDataType& dataType, - const Types::ChannelVector& channelVector, - const std::string& description, - const SizeArray& dsetSize, - const SizeArray& chunkSize, - const float& conversion, - const float& resolution, - const float& offset) + const Types::ChannelVector& channelVector, + const std::string& description, + const SizeArray& dsetSize, + const SizeArray& chunkSize, + const float& conversion, + const float& resolution, + const float& offset) { TimeSeries::initialize(dataType, "volts", @@ -43,7 +43,8 @@ Status ElectricalSeries::initialize(const IO::BaseDataType& dataType, this->m_channelVector = channelVector; // get the number of electrodes from the electrode table - std::string idPath = AQNWB::mergePaths(ElectrodeTable::electrodeTablePath, "id"); + std::string idPath = + AQNWB::mergePaths(ElectrodeTable::electrodeTablePath, "id"); std::vector elecTableDsetSize = m_io->getDatasetSize(idPath); SizeType numElectrodes = elecTableDsetSize[0]; @@ -53,8 +54,9 @@ Status ElectricalSeries::initialize(const IO::BaseDataType& dataType, for (size_t i = 0; i < channelVector.size(); ++i) { SizeType globalIndex = channelVector[i].getGlobalIndex(); if (globalIndex >= numElectrodes) { - std::cerr << "ElectricalSeries::initialize electrode index " << globalIndex - << " is out of range. Max index is " << (numElectrodes - 1) << std::endl; + std::cerr << "ElectricalSeries::initialize electrode index " + << globalIndex << " is out of range. Max index is " + << (numElectrodes - 1) << std::endl; return Status::Failure; } electrodeInds[i] = static_cast(channelVector[i].getGlobalIndex()); diff --git a/src/nwb/ecephys/ElectricalSeries.hpp b/src/nwb/ecephys/ElectricalSeries.hpp index e2ea32af..98f00005 100644 --- a/src/nwb/ecephys/ElectricalSeries.hpp +++ b/src/nwb/ecephys/ElectricalSeries.hpp @@ -54,13 +54,13 @@ class ElectricalSeries : public TimeSeries */ Status initialize(const IO::BaseDataType& dataType, - const Types::ChannelVector& channelVector, - const std::string& description, - const SizeArray& dsetSize, - const SizeArray& chunkSize, - const float& conversion = 1.0f, - const float& resolution = -1.0f, - const float& offset = 0.0f); + const Types::ChannelVector& channelVector, + const std::string& description, + const SizeArray& dsetSize, + const SizeArray& chunkSize, + const float& conversion = 1.0f, + const float& resolution = -1.0f, + const float& offset = 0.0f); /** * @brief Writes a channel to an ElectricalSeries dataset. diff --git a/tests/examples/testWorkflowExamples.cpp b/tests/examples/testWorkflowExamples.cpp index a3eb5a10..75a3f7ca 100644 --- a/tests/examples/testWorkflowExamples.cpp +++ b/tests/examples/testWorkflowExamples.cpp @@ -52,9 +52,10 @@ TEST_CASE("workflowExamples") // [example_workflow_nwbfile_snippet] // [example_workflow_electrodes_table_snippet] - Status elecTableStatus =nwbfile->createElectrodesTable(mockRecordingArrays); + Status elecTableStatus = + nwbfile->createElectrodesTable(mockRecordingArrays); REQUIRE(elecTableStatus == Status::Success); - // [example_workflow_electrodes_table_snippet] + // [example_workflow_electrodes_table_snippet] // [example_workflow_datasets_snippet] std::vector containerIndexes; diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 44c4f339..f7cf3c3e 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -76,9 +76,10 @@ TEST_CASE("createElectrodesTable", "[nwb]") REQUIRE(resultCreate == Status::Success); } -TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") +TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") { - std::string filename = getTestFilePath("createElectricalSeriesWithSubset.nwb"); + std::string filename = + getTestFilePath("createElectricalSeriesWithSubset.nwb"); // initialize nwbfile object and create base structure std::shared_ptr io = @@ -94,8 +95,10 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") // Create electrical series with subset of electrodes (2 channels) SizeType numChannels = 2; - std::vector recordingElectrodes = getMockChannelArrays(numChannels, 1); - std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + std::vector recordingElectrodes = + getMockChannelArrays(numChannels, 1); + std::vector recordingNames = + getMockChannelArrayNames("esdata", 1); std::unique_ptr recordingContainers = std::make_unique(); Status resultCreateES = @@ -116,11 +119,14 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") for (size_t i = 0; i < recordingElectrodes.size(); ++i) { for (size_t j = 0; j < recordingElectrodes[i].size(); ++j) { - recordingContainers->writeElectricalSeriesData( - 0, recordingElectrodes[i][j], mockData.size(), mockData.data(), mockTimestamps.data()); + recordingContainers->writeElectricalSeriesData(0, + recordingElectrodes[i][j], + mockData.size(), + mockData.data(), + mockTimestamps.data()); } } - + nwbfile.finalize(); } @@ -136,8 +142,10 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") nwbfile.initialize(generateUuid()); // Attempt to create electrical series without creating electrodes table first - std::vector recordingElectrodes = getMockChannelArrays(1, 2); - std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + std::vector recordingElectrodes = + getMockChannelArrays(1, 2); + std::vector recordingNames = + getMockChannelArrayNames("esdata", 1); Status resultCreateES = nwbfile.createElectricalSeries( recordingElectrodes, recordingNames, BaseDataType::F32); REQUIRE(resultCreateES == Status::Failure); @@ -147,7 +155,8 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") { - std::string filename = getTestFilePath("createElectricalSeriesOutOfRange.nwb"); + std::string filename = + getTestFilePath("createElectricalSeriesOutOfRange.nwb"); // initialize nwbfile object and create base structure std::shared_ptr io = @@ -157,21 +166,23 @@ TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") nwbfile.initialize(generateUuid()); // Create electrode table with 2 channels - std::vector tableElectrodes = getMockChannelArrays(2, 1); + std::vector tableElectrodes = + getMockChannelArrays(2, 1); Status resultCreateTable = nwbfile.createElectrodesTable(tableElectrodes); REQUIRE(resultCreateTable == Status::Success); // Attempt to create electrical series with channels having higher indices // Create mock channels with global indices > 1 (out of range of table) - std::vector recordingElectrodes = getMockChannelArrays(4, 1); + std::vector recordingElectrodes = + getMockChannelArrays(4, 1); - std::vector recordingNames = getMockChannelArrayNames("esdata", 1); + std::vector recordingNames = + getMockChannelArrayNames("esdata", 1); Status resultCreateES = nwbfile.createElectricalSeries( recordingElectrodes, recordingNames, BaseDataType::F32); REQUIRE(resultCreateES == Status::Failure); } - TEST_CASE("createElectricalSeries", "[nwb]") { std::string filename = getTestFilePath("createElectricalSeries.nwb"); From 6146a720aaa47f1a269fe7f6f6161664f56494d6 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 20 Feb 2025 09:32:34 -0800 Subject: [PATCH 08/16] use dims from dataset --- src/io/BaseIO.hpp | 4 +--- src/io/hdf5/HDF5IO.cpp | 6 +++--- src/io/hdf5/HDF5IO.hpp | 4 +--- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index bc6950e2..8490e6f2 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -444,11 +444,9 @@ class BaseIO /** * @brief Returns the size of the dataset for each dimension. * @param path The location in the file of the dataset. - * @param ndims The number of dimensions in the dataset, defaults to 1. * @return The dataset size. */ - virtual std::vector getDatasetSize(const std::string path, - const size_t ndims = 1) = 0; + virtual std::vector getDatasetSize(const std::string path) = 0; /** * @brief Convenience function for creating NWB related attributes. diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index fa09aebf..14283046 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -1106,12 +1106,12 @@ HDF5IO::getStorageObjects(const std::string& path, return objects; } -std::vector HDF5IO::getDatasetSize(const std::string path, - const size_t ndims) +std::vector HDF5IO::getDatasetSize(const std::string path) { H5::DataSet dataset = m_file->openDataSet(path); H5::DataSpace dataspace = dataset.getSpace(); - std::vector dims(ndims); + const int rank = dataspace.getSimpleExtentNdims(); + std::vector dims(static_cast(rank)); dataspace.getSimpleExtentDims(dims.data()); return std::vector(dims.begin(), dims.end()); diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index 8879b4a1..cc33b17a 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -271,11 +271,9 @@ class HDF5IO : public BaseIO /** * @brief Returns the size of the dataset for each dimension. * @param path The location in the file of the dataset. - * @param ndims The number of dimensions in the dataset, defaults to 1. * @return The dataset size. */ - std::vector getDatasetSize(const std::string path, - const size_t ndims = 1) override; + std::vector getDatasetSize(const std::string path) override; /** * @brief Checks whether a Dataset, Group, or Link already exists at the From ffdcde3cc9248d641d3da852e3aa2d6c748ddb98 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:28:02 -0800 Subject: [PATCH 09/16] Update docs/pages/userdocs/workflow.dox Co-authored-by: Oliver Ruebel --- docs/pages/userdocs/workflow.dox | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/pages/userdocs/workflow.dox b/docs/pages/userdocs/workflow.dox index 4b9e980a..68a0a0b6 100644 --- a/docs/pages/userdocs/workflow.dox +++ b/docs/pages/userdocs/workflow.dox @@ -14,8 +14,9 @@ * used for managing \ref AQNWB::NWB::Container "Container" objects for storing recordings. * 3. Create the \ref AQNWB::NWB::NWBFile "NWBFile" object used for managing and creating NWB * file contents. - * 4. Create the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object used for saving - * electrodes information. + * 4. Create the recording metadata stored in ``\general`` in NWB, e.g., the + * \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object used for + * saving electrodes information. * 5. Create the \ref AQNWB::NWB::Container "Container" objects (e.g., * \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries") used for recording and add them * to the \ref AQNWB::NWB::RecordingContainers "RecordingContainers". From 4c4ce2d7fe2cb5b908d77cde469d02c113cbad41 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:40:31 -0800 Subject: [PATCH 10/16] Update docs/pages/userdocs/workflow.dox Co-authored-by: Oliver Ruebel --- docs/pages/userdocs/workflow.dox | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/pages/userdocs/workflow.dox b/docs/pages/userdocs/workflow.dox index 68a0a0b6..fdc0a73e 100644 --- a/docs/pages/userdocs/workflow.dox +++ b/docs/pages/userdocs/workflow.dox @@ -55,13 +55,14 @@ * \snippet tests/examples/testWorkflowExamples.cpp example_workflow_nwbfile_snippet * * - * \subsection create_nwbfile 4. Create the ElectrodesTable object. - * + * \subsection create_recmeta 4. Create the recording metadata + * \subsubsection create_recmeta_ecephys Create the extracellular recording metadata * Next, constructs the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object, * using the `recordingArrays` as an input. Note that in many cases, you will be recording from * all electrodes detected by the device and this `recordingArrays` structure will be the same as * is used to create the \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries". However in some cases - * (e.g. Neuropixels), you may be recording from a subset of all available electrodes. In this case, + * (e.g. Neuropixels or when using multiple probees), you may be recording from a subset of + * all available electrodes as part of a single \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries". In this case, * you should create the ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" with all electrodes * detected by the acquisition system, not just those being actively recorded from. This approach is * critical for mapping information about which \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries" From 410da4ad25ce6843be057dfa976e46fa73bd48d2 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 15:00:58 -0800 Subject: [PATCH 11/16] update test filenames for non nwb files --- tests/testNWBFile.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index f7cf3c3e..75b2cc7b 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -113,7 +113,7 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") REQUIRE(resultStart == Status::Success); std::vector mockData = {1.0f, 2.0f, 3.0f}; - std::vector mockTimestamps = {0.1, 0.2, 0.3}; + std::vector mockTimestamps = {0.1, 0.2, 0.4}; std::vector positionOffset = {0, 0}; std::vector dataShape = {mockData.size(), 0}; @@ -132,7 +132,7 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") { - std::string filename = getTestFilePath("createElectricalSeriesNoTable.nwb"); + std::string filename = getTestFilePath("createElectricalSeriesNoTable.h5"); // initialize nwbfile object and create base structure std::shared_ptr io = @@ -156,7 +156,7 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") { std::string filename = - getTestFilePath("createElectricalSeriesOutOfRange.nwb"); + getTestFilePath("createElectricalSeriesOutOfRange.h5"); // initialize nwbfile object and create base structure std::shared_ptr io = From 64552655bc7e53339feb3b82a6c4fd431266208c Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:43:47 -0800 Subject: [PATCH 12/16] update getShape function for attributes and datasets --- src/io/BaseIO.hpp | 5 +++-- src/io/hdf5/HDF5IO.cpp | 13 ++++++++++--- src/io/hdf5/HDF5IO.hpp | 4 ++-- src/nwb/ecephys/ElectricalSeries.cpp | 2 +- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index 8490e6f2..8e0a3166 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -444,9 +444,10 @@ class BaseIO /** * @brief Returns the size of the dataset for each dimension. * @param path The location in the file of the dataset. - * @return The dataset size. + * @return The dataset shape. */ - virtual std::vector getDatasetSize(const std::string path) = 0; + virtual std::vector getStorageObjectShape( + const std::string path) = 0; /** * @brief Convenience function for creating NWB related attributes. diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index 14283046..bc644d5d 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -1106,10 +1106,17 @@ HDF5IO::getStorageObjects(const std::string& path, return objects; } -std::vector HDF5IO::getDatasetSize(const std::string path) +std::vector HDF5IO::getStorageObjectShape(const std::string path) { - H5::DataSet dataset = m_file->openDataSet(path); - H5::DataSpace dataspace = dataset.getSpace(); + H5::DataSpace dataspace; + try { + H5::DataSet dataset = m_file->openDataSet(path); + dataspace = dataset.getSpace(); + } catch (H5::Exception& e) { + // Read the attribute + std::unique_ptr attributePtr = this->getAttribute(path); + dataspace = attributePtr->getSpace(); + } const int rank = dataspace.getSimpleExtentNdims(); std::vector dims(static_cast(rank)); dataspace.getSimpleExtentDims(dims.data()); diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index cc33b17a..8d9dacd2 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -271,9 +271,9 @@ class HDF5IO : public BaseIO /** * @brief Returns the size of the dataset for each dimension. * @param path The location in the file of the dataset. - * @return The dataset size. + * @return The dataset shape. */ - std::vector getDatasetSize(const std::string path) override; + std::vector getStorageObjectShape(const std::string path) override; /** * @brief Checks whether a Dataset, Group, or Link already exists at the diff --git a/src/nwb/ecephys/ElectricalSeries.cpp b/src/nwb/ecephys/ElectricalSeries.cpp index 5f0a9fe8..e68df108 100644 --- a/src/nwb/ecephys/ElectricalSeries.cpp +++ b/src/nwb/ecephys/ElectricalSeries.cpp @@ -45,7 +45,7 @@ Status ElectricalSeries::initialize(const IO::BaseDataType& dataType, // get the number of electrodes from the electrode table std::string idPath = AQNWB::mergePaths(ElectrodeTable::electrodeTablePath, "id"); - std::vector elecTableDsetSize = m_io->getDatasetSize(idPath); + std::vector elecTableDsetSize = m_io->getStorageObjectShape(idPath); SizeType numElectrodes = elecTableDsetSize[0]; // setup variables based on number of channels From 4c7103071fb38a4be43d151b5c0d004774a30315 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:44:15 -0800 Subject: [PATCH 13/16] add tests and fix attr array write --- src/io/hdf5/HDF5IO.cpp | 7 +++--- tests/testHDF5IO.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index bc644d5d..20614fc3 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -653,16 +653,17 @@ Status HDF5IO::createAttribute(const IO::BaseDataType& type, H5type = getH5Type(type); origType = getNativeType(type); + DataSpace attr_dataspace; if (size > 1) { hsize_t dims = static_cast(size); - H5type = ArrayType(H5type, 1, &dims); - origType = ArrayType(origType, 1, &dims); + attr_dataspace = DataSpace(1, &dims); // Create 1D dataspace of size 'dims' + } else { + attr_dataspace = H5S_SCALAR; } if (loc->attrExists(name)) { attr = loc->openAttribute(name); } else { - DataSpace attr_dataspace(H5S_SCALAR); attr = loc->createAttribute(name, H5type, attr_dataspace); } diff --git a/tests/testHDF5IO.cpp b/tests/testHDF5IO.cpp index 10e72958..c1b5c312 100644 --- a/tests/testHDF5IO.cpp +++ b/tests/testHDF5IO.cpp @@ -290,6 +290,55 @@ TEST_CASE("getStorageObjects", "[hdf5io]") } // END TEST_CASE("getStorageObjects", "[hdf5io]") +TEST_CASE("getStorageObjectShape", "[hdf5io]") +{ + // create and open file + std::string filename = getTestFilePath("testGetStorageObjectShape.h5"); + IO::HDF5::HDF5IO hdf5io(filename); + hdf5io.open(); + + SECTION("get shape of a dataset") + { + // Create a 2D dataset + SizeArray dims = {3, 4}; + std::string dataPath = "/dataset2D"; + hdf5io.createArrayDataSet( + BaseDataType::I32, dims, SizeArray {0, 0}, dataPath); + + // Get and verify shape + auto shape = hdf5io.getStorageObjectShape(dataPath); + REQUIRE(shape.size() == 2); + REQUIRE(shape[0] == 3); + REQUIRE(shape[1] == 4); + } + + SECTION("get shape of an attribute") + { + const std::string groupPath = "/data"; + hdf5io.createGroup(groupPath); + + // Test 1D array attribute + const std::vector data = { + 1, + 2, + 3, + 4, + 5, + }; + const std::string attrName = "int_array"; + const std::string attrPath = mergePaths(groupPath, attrName); + + hdf5io.createAttribute( + BaseDataType::I32, data.data(), groupPath, attrName, data.size()); + auto shape = hdf5io.getStorageObjectShape(attrPath); + REQUIRE(shape.size() == 1); + REQUIRE(shape[0] == 5); + } + + // close file + hdf5io.close(); +} + TEST_CASE("HDF5IO; write datasets", "[hdf5io]") { std::vector testData = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; From ac123bd9c1f05a095c34afd7abe9fb441a7438e6 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:44:54 -0800 Subject: [PATCH 14/16] remove static containerIndices vector and require --- src/nwb/NWBFile.cpp | 2 -- src/nwb/NWBFile.hpp | 20 +++++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 062b2943..2a4f3ff5 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -28,8 +28,6 @@ constexpr SizeType CHUNK_XSIZE = constexpr SizeType SPIKE_CHUNK_XSIZE = 8; // TODO - replace with io settings input -std::vector NWBFile::m_emptyContainerIndexes = {}; - // Initialize the static registered_ member to trigger registration REGISTER_SUBCLASS_IMPL(NWBFile) diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index c8feaf4c..830a93c5 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -126,9 +126,9 @@ class NWBFile : public Container Status createElectricalSeries( std::vector recordingArrays, std::vector recordingNames, - const IO::BaseDataType& dataType = IO::BaseDataType::I16, - RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = m_emptyContainerIndexes); + const IO::BaseDataType& dataType, + RecordingContainers* recordingContainers, + std::vector& containerIndexes); /** * @brief Create SpikeEventSeries objects to record data into. @@ -147,9 +147,9 @@ class NWBFile : public Container Status createSpikeEventSeries( std::vector recordingArrays, std::vector recordingNames, - const IO::BaseDataType& dataType = IO::BaseDataType::I16, - RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = m_emptyContainerIndexes); + const IO::BaseDataType& dataType, + RecordingContainers* recordingContainers, + std::vector& containerIndexes); /** @brief Create AnnotationSeries objects to record data into. * Created objects are stored in recordingContainers. @@ -160,10 +160,9 @@ class NWBFile : public Container * recordingContainers * @return Status The status of the object creation operation. */ - Status createAnnotationSeries( - std::vector recordingNames, - RecordingContainers* recordingContainers = nullptr, - std::vector& containerIndexes = m_emptyContainerIndexes); + Status createAnnotationSeries(std::vector recordingNames, + RecordingContainers* recordingContainers, + std::vector& containerIndexes); DEFINE_REGISTERED_FIELD(readElectrodeTable, ElectrodeTable, @@ -261,7 +260,6 @@ class NWBFile : public Container specVariables); inline const static std::string m_acquisitionPath = "/acquisition"; - static std::vector m_emptyContainerIndexes; /** * @brief The ElectrodeTable for the file From 3c0b98e5388b7514607ae03e2a1e014649a34070 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:45:05 -0800 Subject: [PATCH 15/16] update tests --- tests/examples/test_ecephys_data_read.cpp | 9 +++- tests/testNWBFile.cpp | 52 +++++++++++++++++------ tests/testRecordingWorkflow.cpp | 4 +- 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/tests/examples/test_ecephys_data_read.cpp b/tests/examples/test_ecephys_data_read.cpp index 867de663..aca79e8c 100644 --- a/tests/examples/test_ecephys_data_read.cpp +++ b/tests/examples/test_ecephys_data_read.cpp @@ -65,11 +65,16 @@ TEST_CASE("ElectricalSeriesReadExample", "[ecephys]") // create the RecordingContainer for managing recordings std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; // create a new ElectricalSeries nwbfile.createElectrodesTable(mockArrays); - Status resultCreate = nwbfile.createElectricalSeries( - mockArrays, mockChannelNames, dataType, recordingContainers.get()); + Status resultCreate = + nwbfile.createElectricalSeries(mockArrays, + mockChannelNames, + dataType, + recordingContainers.get(), + containerIndices); REQUIRE(resultCreate == Status::Success); // get the new ElectricalSeries diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 75b2cc7b..03973106 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -101,11 +101,13 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") getMockChannelArrayNames("esdata", 1); std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; Status resultCreateES = nwbfile.createElectricalSeries(recordingElectrodes, recordingNames, BaseDataType::F32, - recordingContainers.get()); + recordingContainers.get(), + containerIndices); REQUIRE(resultCreateES == Status::Success); // Write some test data to verify recording works @@ -146,8 +148,15 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") getMockChannelArrays(1, 2); std::vector recordingNames = getMockChannelArrayNames("esdata", 1); - Status resultCreateES = nwbfile.createElectricalSeries( - recordingElectrodes, recordingNames, BaseDataType::F32); + std::unique_ptr recordingContainers = + std::make_unique(); + std::vector containerIndices = {}; + Status resultCreateES = + nwbfile.createElectricalSeries(recordingElectrodes, + recordingNames, + BaseDataType::F32, + recordingContainers.get(), + containerIndices); REQUIRE(resultCreateES == Status::Failure); nwbfile.finalize(); @@ -155,8 +164,7 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") { - std::string filename = - getTestFilePath("createElectricalSeriesOutOfRange.h5"); + std::string filename = getTestFilePath("createElectricalSeriesOutOfRange.h5"); // initialize nwbfile object and create base structure std::shared_ptr io = @@ -178,8 +186,15 @@ TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") std::vector recordingNames = getMockChannelArrayNames("esdata", 1); - Status resultCreateES = nwbfile.createElectricalSeries( - recordingElectrodes, recordingNames, BaseDataType::F32); + std::unique_ptr recordingContainers = + std::make_unique(); + std::vector containerIndices = {}; + Status resultCreateES = + nwbfile.createElectricalSeries(recordingElectrodes, + recordingNames, + BaseDataType::F32, + recordingContainers.get(), + containerIndices); REQUIRE(resultCreateES == Status::Failure); } @@ -203,11 +218,13 @@ TEST_CASE("createElectricalSeries", "[nwb]") getMockChannelArrayNames("esdata"); std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; Status resultCreate = nwbfile.createElectricalSeries(mockArrays, mockChannelNames, BaseDataType::F32, - recordingContainers.get()); + recordingContainers.get(), + containerIndices); REQUIRE(resultCreate == Status::Success); // start recording @@ -273,11 +290,13 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") getMockChannelArrayNames("esdata"); std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; Status resultCreateES = nwbfile.createElectricalSeries(mockArrays, mockChannelNames, BaseDataType::F32, - recordingContainers.get()); + recordingContainers.get(), + containerIndices); REQUIRE(resultCreateES == Status::Success); // create SpikeEventSeries @@ -288,7 +307,8 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") nwbfile.createSpikeEventSeries(mockArrays, mockSpikeChannelNames, BaseDataType::F32, - recordingContainers.get()); + recordingContainers.get(), + containerIndices); REQUIRE(resultCreateSES == Status::Success); // start recording @@ -344,8 +364,9 @@ TEST_CASE("createAnnotationSeries", "[nwb]") "annotations2"}; std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; Status resultCreate = nwbfile.createAnnotationSeries( - mockAnnotationNames, recordingContainers.get()); + mockAnnotationNames, recordingContainers.get(), containerIndices); REQUIRE(resultCreate == Status::Success); // start recording @@ -409,8 +430,13 @@ TEST_CASE("setCanModifyObjectsMode", "[nwb]") std::vector mockChannelNames = getMockChannelArrayNames("esdata"); nwbfile.createElectrodesTable(mockArrays); // create the Electrodes Table - Status resultCreatePostStart = nwbfile.createElectricalSeries( - mockArrays, mockChannelNames, BaseDataType::F32); + std::vector containerIndices = {}; + Status resultCreatePostStart = + nwbfile.createElectricalSeries(mockArrays, + mockChannelNames, + BaseDataType::F32, + nullptr, + containerIndices); REQUIRE(resultCreatePostStart == Status::Failure); // stop recording diff --git a/tests/testRecordingWorkflow.cpp b/tests/testRecordingWorkflow.cpp index aa581811..70c0edd7 100644 --- a/tests/testRecordingWorkflow.cpp +++ b/tests/testRecordingWorkflow.cpp @@ -43,6 +43,7 @@ TEST_CASE("writeContinuousData", "[recording]") // 2. create RecordingContainers object std::unique_ptr recordingContainers = std::make_unique(); + std::vector containerIndices = {}; // 3. create NWBFile object std::unique_ptr nwbfile = std::make_unique(io); @@ -55,7 +56,8 @@ TEST_CASE("writeContinuousData", "[recording]") nwbfile->createElectricalSeries(mockRecordingArrays, mockChannelNames, BaseDataType::F32, - recordingContainers.get()); + recordingContainers.get(), + containerIndices); // 6. start the recording io->startRecording(); From e3ef7132af341cdb0ef36e74630782d038dcb9e6 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:46:27 -0800 Subject: [PATCH 16/16] fix typos in workflow docs --- docs/pages/userdocs/workflow.dox | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/pages/userdocs/workflow.dox b/docs/pages/userdocs/workflow.dox index fdc0a73e..b4f3df85 100644 --- a/docs/pages/userdocs/workflow.dox +++ b/docs/pages/userdocs/workflow.dox @@ -57,11 +57,11 @@ * * \subsection create_recmeta 4. Create the recording metadata * \subsubsection create_recmeta_ecephys Create the extracellular recording metadata - * Next, constructs the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object, + * Next, construct the \ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" object, * using the `recordingArrays` as an input. Note that in many cases, you will be recording from * all electrodes detected by the device and this `recordingArrays` structure will be the same as * is used to create the \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries". However in some cases - * (e.g. Neuropixels or when using multiple probees), you may be recording from a subset of + * (e.g. when using Neuropixels or multiple probes), you may be recording from a subset of * all available electrodes as part of a single \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries". In this case, * you should create the ref AQNWB::NWB::ElectrodesTable "ElectrodesTable" with all electrodes * detected by the acquisition system, not just those being actively recorded from. This approach is