update methods for writing n-dimensional data

[stephprince]

• Fix Extend BaseRecordingData to write n-dimensional data #6
• Fix update BaseRecordingData class for multiple dimensions #11

These changes update the writeDataBlock method of BaseRecordingData for n-dimensional data and add methods for managing writing data as part of the TimeSeries and ElectricalSeries classes. This include a new RecordingContainer class to hold groups of TimeSeries objects for searching and indexing during the recording process.

As part of these changes I also:

• added detection of the type (dataset/group) of a path in the HDF5 file to prevent HDF5 errors
• set the default TimeSeries timestamps datatype to float64 as specified in the schema
• separated any data transformations from the write methods
• updated the base TimeSeries constructor

[stephprince]

Some of the tests are failing I believe due to different versions of hdf5 available on the different platforms, specifically H5Oget_info_by_name takes a different numbers of arguments in 1.10 vs 1.14. Maybe we can discuss further whether we want to adapt the code or use a specific version of hdf5

[oruebel]

The main code looks good to me. I added a few questions and comments, but I don't think there is anything that should require large changes. I still need to review the tests.

[stephprince]

I made some updates and removed the recordingContainers tags functionality for now, not sure if we want to address that here or in a separate PR.

I'm going to try debugging the failing tests still, I'll ping you if I have an update.

[oruebel]

I made some updates and removed the recordingContainers tags functionality for now, not sure if we want to address that here or in a separate PR.

I think a separate Issue/PR is fine. I don't think tags are essential for the purpose of this PR.

I'm going to try debugging the failing tests still, I'll ping you if I have an update.

Sounds good 👍

[oruebel]

Aside from the failing tests, this PR looks good to me.

[stephprince]

would it be ok if we merged for now and I fix the failing tests in a separate issue? I'm not sure how to best approach debugging those.

[oruebel]

would it be ok if we merged for now and I fix the failing tests in a separate issue? I'm not sure how to best approach debugging those.

Sure, debugging this in a separate PR is fine if that makes things easier.

Are the tests also failing on your labtop or only in the Ubuntu environment?

For the segfaults, a first step would be to change the test pipeline to compile in DEBUG mode to get a more detailed traceback for the segfaults, which should hopefully help find the place where things go wrong. If that doesn't help you find the source of the error, then I would look at valgrind https://valgrind.org/docs/manual/quick-start.html#quick-start.prepare to help find memory issues.

For the error where REQUIRE( tsRead == zeros ) fails, I'm not sure I understand this test:

aqnwb/tests/testBase.cpp

Lines 60 to 82 in c71ec22

	SECTION("test writing timeseries without timestamps")
	{
	// setup timeseries object
	std::string path = getTestFilePath("testTimeseriesNoTimestamps.h5");
	std::shared_ptr<BaseIO> io = createIO("HDF5", path);
	io->open();
	NWB::TimeSeries ts = NWB::TimeSeries(dataPath, io, dataType, "unit");
	ts.initialize();
	// Write data to file
	Status writeStatus = ts.writeData(dataShape, positionOffset, data.data());
	REQUIRE(writeStatus == Status::Success);
	// Read data back from file
	double* tsBuffer = new double[numSamples];
	BaseRecordingData* tsDset = io->getDataSet(dataPath + "/timestamps");
	readH5DataBlock(static_cast<HDF5::HDF5RecordingData*>(tsDset)->getDataSet(),
	timestampsType,
	tsBuffer);
	std::vector<double> tsRead(tsBuffer, tsBuffer + numSamples);
	delete[] tsBuffer;
	std::vector<double> zeros(numSamples, 0.0);
	REQUIRE(tsRead == zeros);

The test says it is writing a TimeSeries without timestamps, but then it compares the values of timestamps? Also, if timestamps are not set, then we should have a starting_time and sampling_rate.