Feature support for NWBv2 #179

t-b · 2019-05-30T13:56:49Z

Cross links

#22
#179 (TODO list)
AllenInstitute/IPNWB#5
AllenInstitute/IPNWB#16
t-b/ndx-MIES#1

Requires:

Support NWB schema 2.2.2 NeurodataWithoutBorders/pynwb#1146

Issues:

icephys.yaml: Some entries miss unit attribute NeurodataWithoutBorders/nwb-schema#410

TODOs:

Close #22.

t-b · 2019-05-30T17:15:19Z

@ukos-git Failed NWBv2 export: PXP and NWB
HardwareTests.zip

BasicHardwareTests#UnassociatedChannels_REENTRY(str="ITC18USB_DEV_0")

t-b · 2019-05-30T17:34:13Z

@ukos-git The worst bug is fixed in cf57690.
The tests still don't pass.

I think we should:

write "PLACEHOLDER" instead of "" for unassociated AD channels (AD 2)
Fix the stimulus_description for "DA 2"

ukos-git · 2019-05-31T07:45:54Z

cf57690 should be done like the others above with IPNWB#GetNWBgroupPatchClampSeries(nwbVersion) I don't like the PLACEHOLDER approach in general. If stimulus_description is mandatory, it should go to write single channel.

…

@ukos-git The worst bug is fixed in . The tests still don't pass. I think we should: write "PLACEHOLDER" instead of "" for unassociated AD channels (AD 2) Fix the stimulus_description for "DA 2" — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

t-b · 2019-05-31T09:12:48Z

@ukos-git

I don't like the PLACEHOLDER approach in general.

The attribute is mandatory but for unassociated AD channels we don't have a stimset. Don't you think that "PLACEHOLDER" is better than empty?

ukos-git · 2019-06-04T15:42:58Z

@t-b further changes to hdmf are required to read the json specifications from our NWB file. Same problem as before.

our format with simple dataspace

{
DATASET "/specifications/core/2.0.1/namespace" {
   DATATYPE  H5T_STRING {
      STRSIZE H5T_VARIABLE;
      STRPAD H5T_STR_NULLTERM;
      CSET H5T_CSET_UTF8;
      CTYPE H5T_C_S1;
   }
   DATASPACE  SIMPLE { ( 1 ) / ( 1 ) }
   DATA {
   (0): "…"
   }
}
}

crashes here

https://github.com/hdmf-dev/hdmf/blob/8ddc8574123853215a4d6c43d27bd2aec908017d/src/hdmf/backends/hdf5/h5_utils.py#L171-L174

ukos-git · 2019-06-04T16:32:30Z

@ukos-git Failed NWBv2 export: PXP and NWB

Fails at the IGORWaveNote Attribute of the acquisition wave. probably need a MWE to further investigate this. Maybe the line breaking using <CR>?

t-b · 2019-06-05T21:20:13Z

@ukos-git Is the latest IPNWB version pushed?

I'm getting here:

From https://github.com/AllenInstitute/IPNWB
 + 44b9cd7...7315b98 feature/nwbv2 -> origin/feature/nwbv2  (forced update)
error: Server does not allow request for unadvertised object 13da2c145a4cbc945f972808050ee0eebdcef74c

ukos-git · 2019-06-06T12:18:20Z

I guess, you refer to a339bcd. Rebase was necessary due to the huge amount of "changes" and "wip" commit messages. We should probably squash all commits per MR and avoid force pushing, as github looses all commentary etc on force pushes.

t-b · 2019-06-06T12:28:02Z

I guess, you refer to a339bcd. Rebase was necessary due to the huge amount of "changes" and "wip" commit messages. We should probably squash all commits per MR and avoid force pushing, as github looses all commentary etc on force pushes.

The submodule commit referenced in the parent repo needs to be available as top commit in one of the branches (in git lingo it needs to be the tip of a branch). This is what the error message is complaining about.

Yes github is still not able to handle force pushes and comments properly. That is the reason I'm mainly commenting with non-commit comments.

ukos-git · 2019-06-06T12:33:37Z

You will probably have to do a hard reset on the MIES repo

▶ git ls-tree feature/nwbv2 IPNWB
160000 commit 7315b98ac805b974a18efe5e5dae0bc131106259	IPNWB
▶ git ls-tree feature/nwbv2~1 IPNWB
160000 commit 13da2c145a4cbc945f972808050ee0eebdcef74c	IPNWB
▶ git ls-tree  a339bcd IPNWB        
160000 commit 13da2c145a4cbc945f972808050ee0eebdcef74c	IPNWB

The current IPNWB commit is over at IPNWB, top commit of branch feature/nwbv2. But a339bcd has an unreferenced (old) commit pointing to 13da2c145a4cbc945f972808050ee0eebdcef74c that should get removed by squashing on merge.

https://github.com/AllenInstitute/IPNWB/commits/feature/nwbv2

ukos-git · 2019-06-06T14:02:25Z

Traceback (most recent call last):
  File "src/pynwb/validate.py", line 117, in <module>
    main()
  File "src/pynwb/validate.py", line 73, in main
    namespaces = HDF5IO.load_namespaces(catalog, path).keys()
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/utils.py", line 388, in func_call
    return func(self, **parsed['args'])
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/backends/hdf5/h5tools.py", line 99, in load_namespaces
    d.update(namespace_catalog.load_namespaces('namespace', reader=reader))
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/utils.py", line 388, in func_call
    return func(self, **parsed['args'])
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/spec/namespace.py", line 447, in load_namespaces
    namespaces = reader.read_namespace(namespace_path)
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/backends/hdf5/h5_utils.py", line 172, in read_namespace
    ret = self.__read(ns_path)
  File "/home/matthias/Documents/Projects/hdmf/src/hdmf/backends/hdf5/h5_utils.py", line 165, in __read
    d = json.loads(s)
  File "/usr/lib/python3.7/json/__init__.py", line 341, in loads
    raise TypeError(f'the JSON object must be str, bytes or bytearray, '
TypeError: the JSON object must be str, bytes or bytearray, not ndarray

t-b · 2019-06-06T21:53:51Z

@ukos-git if you use the latest ipnwb branches the validation against the cached namespace results in the same errors as against the pynwb spec.

ukos-git · 2019-06-07T10:52:56Z

Do you mean the two 64bit integer errors? - I used the pynwb yaml files without changing the spec but we can not write 64bit integers with current HDF5 XOP.

So it is somewhat expected that cached spec and plain pynwb spec give the same errors.

t-b · 2019-06-07T12:41:05Z

@ukos-git Yes these errors are expected. But I plan to tweak the pynwb yaml spec so that our files are compliant as well.

t-b · 2019-06-11T18:22:41Z

@ukos-git I've looked into the stimset storage for the unassociated channels. For ADC we don't have one anyway. So only DAC needs some thinking.
And I think the current behaviour in master is correct. Unassociated channels are stored as plain TimeSeries and as that they have, especially for v2, no associated stimset.

t-b · 2019-10-04T12:22:49Z

Rebased onto latest master.

If that is defined the experiment is never saved.

We want to be notified if a function executing during NWB_ExportAllData raised an RTE.

… old experiments Saved experiments prior to 14b1205 (ExpConfig_ConfigureMIES: Close the user config notebook immediately, 2018-06-26) which used the legacy experiment configuration have the file UserConfig.txt open as notebook. This results in the following experiment recreation macro snippet NewPath MIES ":::Program Files:MIES:" // ... OpenNotebook/N=UserConfigNB/R/W=(5.25,44.75,705,500)/V=0/P=MIES "UserConfig.txt" If one of these experiments is now exported into NWB the user is getting a Missing File/Folder dialog as UserConfig.txt can not be found. But if we just create an empty file UserConfig.txt we can trick Igor Pro into not complaining.

We now check the versions stored in the JSON documents and compare them to the group names in /specifications and the /nwb_version attribute.

In case we got an abort we don't want to keep the NWB file as it is incomplete.

comparison

This avoids an error on later adding more data into the file. See also hdmf-dev/hdmf#354.

When we start a new experiment from the current one we don't want to reuse the old session start time as, not very suprisingly, a new session has begun. Noticed by Wayne Wakeman doing mass conversions to NWBv2.

In the previous commit we fixed resetting the session start time on save and clear. For existing experiments we need to find a workaround on data export. We now ignore session start times which are older (45min) than the oldest sweep.

This function is called from BeforeUncompiledHook and thus needs to always work. Even with really old experiments which don't have the expected GUI controls and thus assert out.

t-b · 2020-06-02T18:31:17Z

@timjarsky Ready to merge according to Wayne.

timjarsky

Big day for MIES.

t-b requested a review from timjarsky as a code owner May 30, 2019 13:56

t-b force-pushed the feature/nwbv2 branch from 1a17c89 to ac7a4b8 Compare May 30, 2019 14:04

t-b force-pushed the feature/nwbv2 branch from cf57690 to d38452e Compare May 31, 2019 09:10

This comment has been minimized.

Sign in to view

ukos-git mentioned this pull request Jun 3, 2019

Porting to NWB v2 AllenInstitute/IPNWB#5

Closed

ukos-git force-pushed the feature/nwbv2 branch from b6009de to a339bcd Compare June 4, 2019 15:35

ukos-git force-pushed the feature/nwbv2 branch from 5b1a51e to a1b41ae Compare June 4, 2019 16:33

t-b assigned t-b and ukos-git Jun 8, 2019

t-b unassigned t-b and ukos-git Oct 2, 2019

t-b force-pushed the feature/nwbv2 branch from e6c4a34 to da9f0c4 Compare October 4, 2019 12:21

t-b force-pushed the feature/nwbv2 branch 2 times, most recently from d804b02 to cc439d9 Compare October 13, 2019 16:42

t-b added 21 commits May 29, 2020 10:55

TestTimeSeries: Pass in the NWB file name instead of guessing it

1d0c5da

UTF_TestNWBExportV1.ipf: Use -V1 suffix for NWB files

989fa8f

Hardware Tests: Handle I=0 timeseries correctly for NWBv2 export tests

ecafe30

Add MIES_PXP_NWB_CONVERSION_SKIP_SAVING define

a18c5db

If that is defined the experiment is never saved.

Add script for mass conversion of PXPs to NWBv2

6dcb717

Update IPNWB

b5caec6

MIES_MassExperimentProcessing.ipf: Assert on lingering RTEs

7b49310

We want to be notified if a function executing during NWB_ExportAllData raised an RTE.

UTF_TestNWBExportV2.ipf: Check the NWB versions

40eeb70

We now check the versions stored in the JSON documents and compare them to the group names in /specifications and the /nwb_version attribute.

ProcessCurrentExperiment: Delete NWB file on error

6d70c92

In case we got an abort we don't want to keep the NWB file as it is incomplete.

Update IPNWB

370ce8a

Update IPNWB

b8824f6

NWBv2 Export: Write neurodata types for ndx-mies extension

e18539b

Testing/NWBv2: Adapt test to translate hardcoded units back for

9216a5e

comparison

MIES_IVSCC: Export files into NWBv2 by default

533ba5c

Update IPNWB

0799224

NWB_AddDeviceSpecificData: Don't add a neurodata type to /general

b3f679e

This avoids an error on later adding more data into the file. See also hdmf-dev/hdmf#354.

Update IPNWB

f9a764e

SaveExperimentSpecial: Reset session start time in "Save and clear" mode

a60f90d

When we start a new experiment from the current one we don't want to reuse the old session start time as, not very suprisingly, a new session has begun. Noticed by Wayne Wakeman doing mass conversions to NWBv2.

DQ_StopOngoingDAQAllLocked: Make it more robust

33b63e3

This function is called from BeforeUncompiledHook and thus needs to always work. Even with really old experiments which don't have the expected GUI controls and thus assert out.

t-b force-pushed the feature/nwbv2 branch from ad0fb26 to 33b63e3 Compare May 29, 2020 17:56

t-b requested a review from timjarsky June 2, 2020 18:30

t-b assigned timjarsky and unassigned t-b Jun 2, 2020

timjarsky approved these changes Jun 5, 2020

View reviewed changes

t-b merged commit 1424f9f into master Jun 5, 2020

t-b deleted the feature/nwbv2 branch June 5, 2020 19:36

t-b mentioned this pull request Jun 5, 2020

DQ_StopOngoingDAQAllLocked: Make it more robust #552

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature support for NWBv2 #179

Feature support for NWBv2 #179

t-b commented May 30, 2019 •

edited

Loading

t-b commented May 30, 2019 •

edited

Loading

t-b commented May 30, 2019

ukos-git commented May 31, 2019 via email

This comment has been minimized.

t-b commented May 31, 2019

ukos-git commented Jun 4, 2019

ukos-git commented Jun 4, 2019 •

edited

Loading

t-b commented Jun 5, 2019

ukos-git commented Jun 6, 2019 •

edited

Loading

t-b commented Jun 6, 2019

ukos-git commented Jun 6, 2019 •

edited

Loading

ukos-git commented Jun 6, 2019

t-b commented Jun 6, 2019

ukos-git commented Jun 7, 2019 •

edited

Loading

t-b commented Jun 7, 2019

t-b commented Jun 11, 2019

t-b commented Oct 4, 2019

t-b commented Jun 2, 2020

timjarsky left a comment

Feature support for NWBv2 #179

Feature support for NWBv2 #179

Conversation

t-b commented May 30, 2019 • edited Loading

Cross links

t-b commented May 30, 2019 • edited Loading

t-b commented May 30, 2019

ukos-git commented May 31, 2019 via email

This comment has been minimized.

t-b commented May 31, 2019

ukos-git commented Jun 4, 2019

ukos-git commented Jun 4, 2019 • edited Loading

t-b commented Jun 5, 2019

ukos-git commented Jun 6, 2019 • edited Loading

t-b commented Jun 6, 2019

ukos-git commented Jun 6, 2019 • edited Loading

ukos-git commented Jun 6, 2019

t-b commented Jun 6, 2019

ukos-git commented Jun 7, 2019 • edited Loading

t-b commented Jun 7, 2019

t-b commented Jun 11, 2019

t-b commented Oct 4, 2019

t-b commented Jun 2, 2020

timjarsky left a comment

Choose a reason for hiding this comment

t-b commented May 30, 2019 •

edited

Loading

t-b commented May 30, 2019 •

edited

Loading

ukos-git commented Jun 4, 2019 •

edited

Loading

ukos-git commented Jun 6, 2019 •

edited

Loading

ukos-git commented Jun 6, 2019 •

edited

Loading

ukos-git commented Jun 7, 2019 •

edited

Loading