Adding unit test for oneepoch #337

jesshaley · 2025-04-28T04:28:19Z

@stevevanhooser having some problems with ingestion of white matter data...

Bugs with reading sample rate in +ndi/+daq/+reader/+mfdaq/ndr.m and +ndi/+daq/+reader/mfdaq.m (see quick fixes in changed files on this branch).
Bugs with handling int16 data. Try running the OneEpochTest. It breaks on S.ingest() due to issues in +ndi/+daq/+reader/mfdaq.m starting with underlying_datatype().

stevevanhooser · 2025-04-28T11:09:11Z

Thanks..there are some changes on main that this branch doesn’t have to ndi.daq.reader.mfdaq.ndr.. If you update it will probably fix the issue Thanks! Steve

…

------------------------------------------ Stephen D. Van Hooser, PhD Associate Professor of Biology Brandeis University (+1) 781-736-2721 {www <http://www.vhlab.org/>,wiki <http://wiki.vhlab.org/>,code <http://code.vhlab.org/>,dataclass <http://dataclass.vhlab.org/>,twitter <http://twitter.vhlab.org/>}.vhlab.org <http://vhlab.org/>

On Apr 28, 2025, at 12:29 AM, Jess Haley ***@***.***> wrote: @jesshaley [github.com] <https://www.google.com/url?q=https://urldefense.com/v3/__https://github.com/jesshaley__;!!DaRZpAeNFA!dNhtapYE3vATICm15aSB6bIpUpVRjrh0mNpCmgucQc_cY98CSRRv6Uj-Qz6mbOAtVlhcn6bffzWjy9WlBnIqlLJ0k-U$&source=gmail-imap&ust=1746419349000000&usg=AOvVaw3AFuMvDBQgB4qVjcRMsV1n> requested your review on: #337 [github.com] <https://www.google.com/url?q=https://urldefense.com/v3/__https://github.com/VH-Lab/NDI-matlab/pull/337__;!!DaRZpAeNFA!dNhtapYE3vATICm15aSB6bIpUpVRjrh0mNpCmgucQc_cY98CSRRv6Uj-Qz6mbOAtVlhcn6bffzWjy9WlBnIqiVuZ5Ao$&source=gmail-imap&ust=1746419349000000&usg=AOvVaw0u9PM6RTXdCg9AGxrO_Sjw> Adding unit test for oneepoch. — Reply to this email directly, view it on GitHub [github.com] <https://www.google.com/url?q=https://urldefense.com/v3/__https://github.com/VH-Lab/NDI-matlab/pull/337*event-17426772459__;Iw!!DaRZpAeNFA!dNhtapYE3vATICm15aSB6bIpUpVRjrh0mNpCmgucQc_cY98CSRRv6Uj-Qz6mbOAtVlhcn6bffzWjy9WlBnIqvfcsx6Q$&source=gmail-imap&ust=1746419349000000&usg=AOvVaw0yUhLahEEmsppm5nUR0t6_>, or unsubscribe [github.com] <https://www.google.com/url?q=https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AAOOPDL5PJ2ICIJHOUX2QED23WVBFAVCNFSM6AAAAAB37ODJGCVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJXGQZDMNZXGI2DKOI__;!!DaRZpAeNFA!dNhtapYE3vATICm15aSB6bIpUpVRjrh0mNpCmgucQc_cY98CSRRv6Uj-Qz6mbOAtVlhcn6bffzWjy9WlBnIqYXHeJuM$&source=gmail-imap&ust=1746419349000000&usg=AOvVaw1_W5Uh8ZPD0y5W5jnZGVA3>. You are receiving this because your review was requested.

Update to latest NDI ndr reader

jesshaley · 2025-04-28T12:49:21Z

Unless I'm doing something wrong, the int16 issue seems to still exist.

Error using  .* 
Integers can only be combined with integers of the same class, or scalar doubles.

Error in ndi.compress.scaled2underlying

Error in ndi.daq.reader.mfdaq/ingest_epochfiles (line 793)
                                data = ndi.compress.scaled2underlying(data, mypoly);
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error in ndi.daq.system/ingest (line 431)
                    new_d = ndi_daqsystem_obj.daqreader.ingest_epochfiles(ef,et(i).epoch_id);
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error in ndi.session/ingest (line 459)
                [b_here,daq_d{i}] = daqs{i}.ingest();

mypoly is a double while data is int16.

NDI-matlab/+ndi/+daq/+reader/mfdaq.m

Line 752 in 59568d2

    
           [underlying_format,mypoly,datasize] = ndi_daqreader_mfdaq_obj.underlying_datatype(epochfiles,...

Still need to reorganize test structure.

Still finishing up documentation and organization of unittest

jesshaley · 2025-04-29T23:03:16Z

@stevevanhooser I'm still wrapping this up, but the unittest revealed an interesting bug. When the data is created in ndi.element.oneepoch, the class matches the input elements. However, after the oneepoch data is added to the element the class is converted to double. Am I doing something wrong? Or is this a bug in ndi.element.timeseries?

% class(data) = 'int16'
elem_out = ndi.element.timeseries(D, name_out, reference_out,...
        ndi_element_timeseries_obj_in.type,ndi_element_timeseries_obj_in,false);
[elem_out,epochdoc] = elem_out.addepoch(epoch_id,...
    strjoin(ecs,','), t0_t1, time, data, strjoin(epoch_ids,','));
[data,time] = elem_out.readtimeseries(1,-inf,inf);
% class(data) = 'double'

jesshaley · 2025-04-30T19:29:45Z

@stevevanhooser I've completed this draft of a OneEpochTest. I'm not sure if I've maintained all the best practices (it seems difficult to find literature on this), but I used fixtures that will be reusable and included some parameterization of test parameters. The unittest still fails while checking the class of the oneepoch elements as mentioned in my previous comment.

stevevanhooser · 2025-04-30T23:27:02Z

tests/+ndi/+unittest/+element/OneEpochTest.m

+                oneepoch_data = oneepoch.readtimeseries(1,-Inf,Inf);
+                testCase.verifySize(oneepoch_data,size(probe_data),...
+                    'OneEpoch does not return the expected number of samples.')
+                testCase.verifyClass(oneepoch_data,class(probe_data),...


This is failing for me, too, for unknown reasons. When I went into the debugger, oneepoch_data is in fact a double. So somehow the command isn't working, I don't quite understand.

But I think a better test might be:

testCase.assertEqual(a, b_expected, ... 'Values should be equal, treating NaNs as equal.', ... 'TreatNaNsAsEqual', true);

This not only tests the class and the size but also whether the values are equal (allowing NaNs to be equal as well; by default any NaN makes equal false).

Does that make sense?

jesshaley · 2025-04-30T23:31:13Z

That makes sense, though may fail if global time is used and epochs are non-continuous. However, shouldn’t oneepoch_data be int16 since white matter data is int16 (and thus probe_data is int16)?

…

On Wed, Apr 30, 2025 at 4:27 PM Steve Van Hooser ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/+ndi/+unittest/+element/OneEpochTest.m <#337 (comment)>: > + probe_et = probe.epochtable; + oneepoch_et = oneepoch.epochtable; + testCase.verifyEqual(oneepoch_et.epoch_clock,probe_et(1).epoch_clock,... + 'OneEpoch clock type(s) do not match those of the original timeseries.') + testCase.verifySize(oneepoch_et,[1,1],... + 'OneEpoch epoch table does not contain a single epoch.') + + % Get data + probe_data = []; + for j = 1:numel(probe_et) + probe_data = cat(1,probe_data,probe.readtimeseries(j,-Inf,Inf)); + end + oneepoch_data = oneepoch.readtimeseries(1,-Inf,Inf); + testCase.verifySize(oneepoch_data,size(probe_data),... + 'OneEpoch does not return the expected number of samples.') + testCase.verifyClass(oneepoch_data,class(probe_data),... This is failing for me, too, for unknown reasons. When I went into the debugger, oneepoch_data is in fact a double. So somehow the command isn't working, I don't quite understand. But I think a better test might be: testCase.assertEqual(a, b_expected, ... 'Values should be equal, treating NaNs as equal.', ... 'TreatNaNsAsEqual', true); This not only tests the class and the size but also whether the values are equal (allowing NaNs to be equal as well; by default any NaN makes equal false). Does that make sense? — Reply to this email directly, view it on GitHub <#337 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJDALDOEPTVEDJZSJ7MK6PD24FL5ZAVCNFSM6AAAAAB37ODJGCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMBYHAYTIMZQHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

stevevanhooser · 2025-04-30T23:46:33Z

Oh I see. oneepoch_data will be double because I think it is stored by addepoch as double when it is written to the binary file:

https://github.com/VH-Lab/NDI-matlab/blob/59568d2bb2128bef4e3058ffdc2380a05b4c3de6/%2Bndi/%2Belement/timeseries.m#L115C13-L115C52

vhsb can use other datatypes but we haven't engaged them (we'd have to pass those parameters with name/value pairs).
https://github.com/VH-Lab/vhlab-toolbox-matlab/blob/2fc776b388e9881f6eee02cdf407a09a2ae80ff2/%2Bvlt/%2Bfile/%2Bcustom_file_formats/vhsb_write.m#L20

So I guess it won't be equal, although you could cast the directly-read probe data to double and then test, since you know element_epochs will be double.

Instead of testing the entire epoch, you could pick a few values that are easy to calculate. If you can easily calculate the value of samples 1500, 1550, 2500, 10000, for example, then you could only test those values, and you might not have to rebuild the whole one epoch vector again from the probe epochs (with the fill-in).

jesshaley · 2025-05-01T15:37:01Z

@stevevanhooser okay! That makes sense. I'll update that code.

stevevanhooser · 2025-05-05T18:45:18Z

Is this ready to be merged?

jesshaley · 2025-05-05T18:47:59Z

Almost. I need to fix the verifyClass issue. I’ll have this done today and will send you a notification.

…

On Mon, May 5, 2025 at 11:45 AM Steve Van Hooser ***@***.***> wrote: *stevevanhooser* left a comment (VH-Lab/NDI-matlab#337) <#337 (comment)> Is this ready to be merged? — Reply to this email directly, view it on GitHub <#337 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJDALDIIQRQAUVSPMVOOP7T246WVHAVCNFSM6AAAAAB37ODJGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNJRHE4TKMRVGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jesshaley · 2025-05-05T22:42:40Z

@stevevanhooser this should be ready to merge now.

stevevanhooser

Thanks! Works on my end

Adding unit test for oneepoch

3a5024a

jesshaley marked this pull request as draft April 28, 2025 04:28

jesshaley requested a review from stevevanhooser April 28, 2025 04:28

Merge pull request #338 from VH-Lab/main

d1a58bc

Update to latest NDI ndr reader

jesshaley added 2 commits April 28, 2025 12:48

Added fixtures and helper functions

2053d38

Still need to reorganize test structure.

New functionality for OneEpochTest

47ab780

Still finishing up documentation and organization of unittest

Completed draft of OneEpochTest and its fixtures

12f3ab6

jesshaley marked this pull request as ready for review April 30, 2025 19:27

stevevanhooser reviewed Apr 30, 2025

View reviewed changes

Swapped verifyClass with verifyEqual

f5bc9b4

stevevanhooser self-requested a review May 5, 2025 23:41

stevevanhooser approved these changes May 5, 2025

View reviewed changes

stevevanhooser merged commit a707bc4 into main May 5, 2025

stevevanhooser deleted the oneepoch-unittest branch May 5, 2025 23:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding unit test for oneepoch #337

Adding unit test for oneepoch #337

Uh oh!

jesshaley commented Apr 28, 2025

Uh oh!

stevevanhooser commented Apr 28, 2025 via email

Uh oh!

jesshaley commented Apr 28, 2025

Uh oh!

jesshaley commented Apr 29, 2025

Uh oh!

jesshaley commented Apr 30, 2025

Uh oh!

stevevanhooser Apr 30, 2025

Uh oh!

jesshaley commented Apr 30, 2025 via email

Uh oh!

stevevanhooser commented Apr 30, 2025

Uh oh!

jesshaley commented May 1, 2025

Uh oh!

stevevanhooser commented May 5, 2025

Uh oh!

jesshaley commented May 5, 2025 via email

Uh oh!

jesshaley commented May 5, 2025

Uh oh!

stevevanhooser left a comment

Uh oh!

Uh oh!

Adding unit test for oneepoch #337

Adding unit test for oneepoch #337

Uh oh!

Conversation

jesshaley commented Apr 28, 2025

Uh oh!

stevevanhooser commented Apr 28, 2025 via email

Uh oh!

jesshaley commented Apr 28, 2025

Uh oh!

jesshaley commented Apr 29, 2025

Uh oh!

jesshaley commented Apr 30, 2025

Uh oh!

stevevanhooser Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

jesshaley commented Apr 30, 2025 via email

Uh oh!

stevevanhooser commented Apr 30, 2025

Uh oh!

jesshaley commented May 1, 2025

Uh oh!

stevevanhooser commented May 5, 2025

Uh oh!

jesshaley commented May 5, 2025 via email

Uh oh!

jesshaley commented May 5, 2025

Uh oh!

stevevanhooser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!