Support Writing Dynamic Filters with DataPipe #428

lawrence-mbf · 2022-05-05T13:30:06Z

Fixes #421

Motivation

Support writing Dynamic Filters with DataPipes.

Checklist

Have you ensured the PR description clearly describes the problem and solutions?
Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

codecov · 2022-05-05T13:37:49Z

Codecov Report

Merging #428 (c40ebf8) into master (1836b93) will decrease coverage by 0.96%.
The diff coverage is 17.10%.

❗ Current head c40ebf8 differs from pull request most recent head 1d17bcc. Consider uploading reports for the commit 1d17bcc to get more accurate results

@@            Coverage Diff             @@
##           master     #428      +/-   ##
==========================================
- Coverage   86.00%   85.04%   -0.97%     
==========================================
  Files         125      126       +1     
  Lines        5067     5195     +128     
==========================================
+ Hits         4358     4418      +60     
- Misses        709      777      +68

Impacted Files	Coverage Δ
...pes/+untyped/+datapipe/+properties/DynamicFilter.m	`0.00% <0.00%> (ø)`
+tests/+unit/dataPipeTest.m	`60.75% <3.22%> (-37.16%)`	⬇️
+types/+untyped/DataPipe.m	`68.54% <66.66%> (-0.36%)`	⬇️
+types/+untyped/+datapipe/BlueprintPipe.m	`75.00% <71.42%> (-0.52%)`	⬇️
+file/fillProps.m	`95.08% <100.00%> (-0.08%)`	⬇️
+types/+util/+dynamictable/nwbToTable.m	`90.00% <0.00%> (-4.60%)`	⬇️
+io/writeCompound.m	`79.16% <0.00%> (-2.32%)`	⬇️
+types/+util/+dynamictable/checkConfig.m	`92.85% <0.00%> (-0.48%)`	⬇️
+tests/+system/DynamicTableTest.m	`89.15% <0.00%> (+0.16%)`	⬆️
+types/+untyped/DataStub.m	`91.90% <0.00%> (+0.27%)`	⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

bendichter · 2022-05-05T13:42:30Z

This looks like a good start. Did you forget to commit other files?

lawrence-mbf · 2022-05-05T13:43:18Z

This looks like a good start. Did you forget to commit other files?

This was supposed to be a draft lol, sorry for having it notify you.

bendichter · 2022-05-05T13:47:39Z

Well looks good so far! Let me know when you have a demo ready

- Dynamic Filter now checks if the filter is available before being created. - The parameters property is now an array like it should be when writing properties. it is still up to the user to format their arguments correctly.

Allows for integration of dynamic filters without using blueprint pipe. Updated tutorials to reflect changes.

azure pipelines now also export the default HDF5 plugins like BZIP2 and others.

lawrence-mbf · 2022-06-03T21:21:36Z

@bendichter I have a barebones tutorial in tutorials/dynamically_loaded_filter_writes.mlx. It currently uses zstd but I think you could flip it to Bzip2 if needed since that's included in hdfplugins by default.

Potential azure pipeline syntax fix.

fix minor variable name error.

tutorials/html/dynamically_loaded_filter_writes.html

- Removed OPTIONAL since some optional properties are actualy readonly. - Fix spaced parentheses for documentation.

Blueprint pipes now check specifically for dynamic filte properties.

Now set to -1 which disables the property.

- Added mention and example for multiple external filters. - Now uses proper NWB example form.

tutorials/html/dynamically_loaded_filter_writes.html

Renamed `externalFilters` -> `extraFilters` On construction, must specify EITHER `compressionLevel` and/or `hasShuffle` OR `extraFilters`. Otherwise, an error will be thrown. Now allows generic Properties as filters to allow for setting default compression or shuffle as extra filters.

Apparantly bzip2 is not provided in the default hdf5plugin package. Changed to LZ4.

For support with external filters array supporting default Compression and Shuffle filters.

Fixed xor case where neither properties are set (shouldn't error). Update consistency of logical flag checks.

Test Heterogeneous filter (shuffle + lz4).

Add extra checks and validations for compressionLevel and hasShuffle properties.

bendichter · 2022-07-21T21:16:55Z

@lawrence-mbf is this ready for review?

lawrence-mbf · 2022-07-21T21:18:50Z

@bendichter Yeah, I updated the conversation in your review but I guess that didn't notify.

+tests/+unit/dataPipeTest.m

tutorials/html/dynamically_loaded_filter_writes.html

The prescence of `filters` now invalidates the existence of other keyword arguments `compressionLevel` and `hasShuffle`.

If a user uses "compressionLevel" and/or "hasShuffle" along with "filters", "filters" wins out and "compressionLevel" and "hasShuffle" are not set at all.

bendichter · 2022-07-22T15:07:27Z

@lawrence-mbf error when running nwbExport on the example livescript:

Error using [hdf5lib2](matlab:matlab.internal.language.introspective.errorDocCallback('hdf5lib2'))
The HDF5 library encountered an error and produced the
following stack trace information:

    H5Z__prelude_callback    error during user callback

Error in [H5D.create](matlab:matlab.internal.language.introspective.errorDocCallback('H5D.create', '/Applications/MATLAB_R2022a.app/toolbox/matlab/imagesci/+H5D/create.m', 63)) ([line 63](matlab: opentoline('/Applications/MATLAB_R2022a.app/toolbox/matlab/imagesci/+H5D/create.m',63,0)))
id = H5ML.hdf5lib2('H5Dcreate', varargin{:} );

Error in [types.untyped.datapipe.BlueprintPipe/write](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.datapipe.BlueprintPipe/write', '/Users/bendichter/dev/matnwb/+types/+untyped/+datapipe/BlueprintPipe.m', 203)) ([line 203](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/+datapipe/BlueprintPipe.m',203,0)))
            did = H5D.create(fid, fullpath, tid, sid, lcpl, dcpl, dapl);

Error in [types.untyped.DataPipe/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.DataPipe/export', '/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m', 274)) ([line 274](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m',274,0)))
            obj.internal = obj.internal.write(fid, fullpath);

Error in [indexing](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.DataPipe/subsref', '/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m', 293)) ([line 293](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m',293,0)))
                B = builtin('subsref', obj, S);

Error in [types.core.TimeSeries/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.core.TimeSeries/export', '/Users/bendichter/dev/matnwb/+types/+core/TimeSeries.m', 340)) ([line 340](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+core/TimeSeries.m',340,0)))
            refs = obj.data.export(fid, [fullpath '/data'], refs);

Error in [types.untyped.Set/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.Set/export', '/Users/bendichter/dev/matnwb/+types/+untyped/Set.m', 181)) ([line 181](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/Set.m',181,0)))
                    refs = v.export(fid, propfp, refs);

Error in [types.core.NWBFile/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.core.NWBFile/export', '/Users/bendichter/dev/matnwb/+types/+core/NWBFile.m', 834)) ([line 834](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+core/NWBFile.m',834,0)))
        refs = obj.acquisition.export(fid, [fullpath '/acquisition'], refs);

Error in [NwbFile/export](matlab:matlab.internal.language.introspective.errorDocCallback('NwbFile/export', '/Users/bendichter/dev/matnwb/NwbFile.m', 62)) ([line 62](matlab: opentoline('/Users/bendichter/dev/matnwb/NwbFile.m',62,0)))
                refs = [email protected](obj, output_file_id, '/', {});

Error in [nwbExport](matlab:matlab.internal.language.introspective.errorDocCallback('nwbExport', '/Users/bendichter/dev/matnwb/nwbExport.m', 36)) ([line 36](matlab: opentoline('/Users/bendichter/dev/matnwb/nwbExport.m',36,0)))
    nwb(i).export(filename);

lawrence-mbf · 2022-07-22T15:15:47Z

@lawrence-mbf error when running nwbExport on the example livescript:

Error using [hdf5lib2](matlab:matlab.internal.language.introspective.errorDocCallback('hdf5lib2'))
The HDF5 library encountered an error and produced the
following stack trace information:

    H5Z__prelude_callback    error during user callback

Error in [H5D.create](matlab:matlab.internal.language.introspective.errorDocCallback('H5D.create', '/Applications/MATLAB_R2022a.app/toolbox/matlab/imagesci/+H5D/create.m', 63)) ([line 63](matlab: opentoline('/Applications/MATLAB_R2022a.app/toolbox/matlab/imagesci/+H5D/create.m',63,0)))
id = H5ML.hdf5lib2('H5Dcreate', varargin{:} );

Error in [types.untyped.datapipe.BlueprintPipe/write](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.datapipe.BlueprintPipe/write', '/Users/bendichter/dev/matnwb/+types/+untyped/+datapipe/BlueprintPipe.m', 203)) ([line 203](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/+datapipe/BlueprintPipe.m',203,0)))
            did = H5D.create(fid, fullpath, tid, sid, lcpl, dcpl, dapl);

Error in [types.untyped.DataPipe/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.DataPipe/export', '/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m', 274)) ([line 274](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m',274,0)))
            obj.internal = obj.internal.write(fid, fullpath);

Error in [indexing](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.DataPipe/subsref', '/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m', 293)) ([line 293](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/DataPipe.m',293,0)))
                B = builtin('subsref', obj, S);

Error in [types.core.TimeSeries/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.core.TimeSeries/export', '/Users/bendichter/dev/matnwb/+types/+core/TimeSeries.m', 340)) ([line 340](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+core/TimeSeries.m',340,0)))
            refs = obj.data.export(fid, [fullpath '/data'], refs);

Error in [types.untyped.Set/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.untyped.Set/export', '/Users/bendichter/dev/matnwb/+types/+untyped/Set.m', 181)) ([line 181](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+untyped/Set.m',181,0)))
                    refs = v.export(fid, propfp, refs);

Error in [types.core.NWBFile/export](matlab:matlab.internal.language.introspective.errorDocCallback('types.core.NWBFile/export', '/Users/bendichter/dev/matnwb/+types/+core/NWBFile.m', 834)) ([line 834](matlab: opentoline('/Users/bendichter/dev/matnwb/+types/+core/NWBFile.m',834,0)))
        refs = obj.acquisition.export(fid, [fullpath '/acquisition'], refs);

Error in [NwbFile/export](matlab:matlab.internal.language.introspective.errorDocCallback('NwbFile/export', '/Users/bendichter/dev/matnwb/NwbFile.m', 62)) ([line 62](matlab: opentoline('/Users/bendichter/dev/matnwb/NwbFile.m',62,0)))
                refs = [email protected](obj, output_file_id, '/', {});

Error in [nwbExport](matlab:matlab.internal.language.introspective.errorDocCallback('nwbExport', '/Users/bendichter/dev/matnwb/nwbExport.m', 36)) ([line 36](matlab: opentoline('/Users/bendichter/dev/matnwb/nwbExport.m',36,0)))
    nwb(i).export(filename);

Oh that's weird. I can't recreate that at all.

lawrence-mbf · 2022-07-22T15:17:57Z

That looks like an error with the plugin itself maybe? I'm currently using the library here: https://github.com/aparamon/HDF5Plugin-Zstandard I don't think hdf5lib actually has this one by default.

Should now show the warning correctly.

bendichter · 2022-07-22T15:31:34Z

@lawrence-mbf are you not using hdf5plugin?

lawrence-mbf · 2022-07-22T15:33:26Z

@bendichter Oh wait no you're right that's what I'm using.

add registered filters enum

6b86d41

lawrence-mbf requested a review from bendichter May 5, 2022 13:30

lawrence-mbf self-assigned this May 5, 2022

lawrence-mbf marked this pull request as draft May 5, 2022 13:30

lawrence-mbf added 6 commits May 17, 2022 11:19

add DynamicFilter property object

bbd5e24

Move DynamicFilter parameters to property

558d50e

Tighten Dynamic Filter security and fix bugs

b74cca4

- Dynamic Filter now checks if the filter is available before being created. - The parameters property is now an array like it should be when writing properties. it is still up to the user to format their arguments correctly.

add basic tutorial

3517e68

Add externalFilter keyword argument for datapipe

0c8d03c

Allows for integration of dynamic filters without using blueprint pipe. Updated tutorials to reflect changes.

Add external filter tests

d7c999f

azure pipelines now also export the default HDF5 plugins like BZIP2 and others.

lawrence-mbf marked this pull request as ready for review June 3, 2022 21:20

lawrence-mbf and others added 3 commits June 3, 2022 17:40

test azure environment path fix

1bf09aa

Update azure-pipelines.yml

9c9adc2

Potential azure pipeline syntax fix.

Update azure-pipelines.yml

82dd978

fix minor variable name error.

bendichter reviewed Jun 10, 2022

View reviewed changes

tutorials/html/dynamically_loaded_filter_writes.html Outdated Show resolved Hide resolved

bendichter reviewed Jun 10, 2022

View reviewed changes

tutorials/html/dynamically_loaded_filter_writes.html Outdated Show resolved Hide resolved

lawrence-mbf added 4 commits June 20, 2022 15:07

Clarify and clean up generated docs

7d5c944

- Removed OPTIONAL since some optional properties are actualy readonly. - Fix spaced parentheses for documentation.

Fix DynamicFilter equality check.

ab287e2

Blueprint pipes now check specifically for dynamic filte properties.

Allow ignoring default datapipe compression

1b55751

Now set to -1 which disables the property.

Update dynamic filter write tutorials

c40ebf8

- Added mention and example for multiple external filters. - Now uses proper NWB example form.

bendichter reviewed Jul 7, 2022

View reviewed changes

tutorials/html/dynamically_loaded_filter_writes.html Outdated Show resolved Hide resolved

lawrence-mbf added 4 commits July 14, 2022 10:56

Try setting env directly in pipeline

76d3590

test tests

0070dd8

disable filter in tests

ce748fb

lawrence-mbf added 7 commits July 14, 2022 12:13

fix incorrect environment variable

84979ad

remove debug error in tests

2b70e9d

Change external filter test to use an included one

7913f0e

Apparantly bzip2 is not provided in the default hdf5plugin package. Changed to LZ4.

DataPipe properties made heterogeneous

5ae3598

For support with external filters array supporting default Compression and Shuffle filters.

Fix DataPipe logic error

9f83aa0

Fixed xor case where neither properties are set (shouldn't error). Update consistency of logical flag checks.

Update external filters test

875326f

Test Heterogeneous filter (shuffle + lz4).

Fix getters/setters with external filters

d456119

Add extra checks and validations for compressionLevel and hasShuffle properties.

bendichter reviewed Jul 21, 2022

View reviewed changes

+tests/+unit/dataPipeTest.m Outdated Show resolved Hide resolved

bendichter reviewed Jul 21, 2022

View reviewed changes

tutorials/html/dynamically_loaded_filter_writes.html Outdated Show resolved Hide resolved

Allow datapipe filters to gain precedence

4d227f8

The prescence of `filters` now invalidates the existence of other keyword arguments `compressionLevel` and `hasShuffle`.

lawrence-mbf requested a review from bendichter July 22, 2022 13:16

Add warning for malformed datapipe keyword use.

c813674

If a user uses "compressionLevel" and/or "hasShuffle" along with "filters", "filters" wins out and "compressionLevel" and "hasShuffle" are not set at all.

Fix dataPipe keyword argument warning

dd1dac5

Should now show the warning correctly.

use shuffle instead of bitshuffle. Clean up docs and use imports

1d17bcc

bendichter approved these changes Jul 22, 2022

View reviewed changes

bendichter merged commit 1afc23c into master Jul 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Writing Dynamic Filters with DataPipe #428

Support Writing Dynamic Filters with DataPipe #428

lawrence-mbf commented May 5, 2022

codecov bot commented May 5, 2022 •

edited

Loading

bendichter commented May 5, 2022

lawrence-mbf commented May 5, 2022

bendichter commented May 5, 2022

lawrence-mbf commented Jun 3, 2022

bendichter commented Jul 21, 2022

lawrence-mbf commented Jul 21, 2022

bendichter commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

bendichter commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

Support Writing Dynamic Filters with DataPipe #428

Support Writing Dynamic Filters with DataPipe #428

Conversation

lawrence-mbf commented May 5, 2022

Motivation

Checklist

codecov bot commented May 5, 2022 • edited Loading

Codecov Report

bendichter commented May 5, 2022

lawrence-mbf commented May 5, 2022

bendichter commented May 5, 2022

lawrence-mbf commented Jun 3, 2022

bendichter commented Jul 21, 2022

lawrence-mbf commented Jul 21, 2022

bendichter commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

bendichter commented Jul 22, 2022

lawrence-mbf commented Jul 22, 2022

codecov bot commented May 5, 2022 •

edited

Loading