Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A loophole #73

Open
Yida-Lin opened this issue Mar 28, 2017 · 11 comments
Open

A loophole #73

Yida-Lin opened this issue Mar 28, 2017 · 11 comments

Comments

@Yida-Lin
Copy link
Contributor

Yida-Lin commented Mar 28, 2017

@cbrnr Just noticed a loophole when debugging Stefan's file:

Before I thought string streams are always irregular streams, but turns out Stefan's file has a stream that is 5000Hz but is string only.

Could also be why that file doesn't load.

I am debugging more into this.

Sorry @stfnrpplngr it might take a while.

@Yida-Lin
Copy link
Contributor Author

Yida-Lin commented Mar 28, 2017

@cbrnr @stfnrpplngr I actually successfully opened this file, though it took quite long loading the file because it has 170,000 events...

I have pushed new commits into SigViewer and Libxdf fixing the loopholes. (in a Pull Requested called fixEventTS, which hasn't been merged into master yet). Now the smaller file that Stefan sent earlier can be successfully opened. The 1.09GB one still can't due to allocation failure (the shortcoming of a 32bit build).

@cbrnr Clemens do you have anything to comment here?

2017-03-28

image

image

image

@cbrnr
Copy link
Owner

cbrnr commented Mar 28, 2017

Is this fix in #71?

@Yida-Lin
Copy link
Contributor Author

@cbrnr Hi Clemens, yes, together with the new commit in libxdf

@cbrnr
Copy link
Owner

cbrnr commented Mar 28, 2017

I still think that converting the original event stream from numeric to string isn't a great idea. However, maybe the easiest solution for us is to treat regular string streams with type sampledMarkers (or whatever the exact name is) as a special case. Specifically, we should remove all empty samples from such a stream and convert only non-empty strings to events.

@Yida-Lin
Copy link
Contributor Author

@cbrnr Hmm I am not sure I exactly understand. Are you saying if the string is empty we should ignore it?

@cbrnr
Copy link
Owner

cbrnr commented Mar 28, 2017

Yes, if the stream type is sampledMarker.

@Yida-Lin
Copy link
Contributor Author

@cbrnr wait, isn't the stream name or type arbitrarily entered in any way the user wants?

Also, if we filter those empty strings out, we won't see those events on the signals and it becomes an empty stream. Is that OK?

@cbrnr
Copy link
Owner

cbrnr commented Mar 28, 2017

Yes, you can choose the type but there are standards for meta data. I'm not sure if this type is in there yet.

This stream should contain some non-empty strings, otherwise what is the point of this signal? So if you discard all empty strings you should be left with the useful ones.

@Yida-Lin
Copy link
Contributor Author

@cbrnr seems the sampledMarker meta standard isn't there yet.

I see. I mean at least in Stefan's test file, it contains only empty strings, and if filtered out, there are no events left at all.

I can go ahead and make the changes, but might need another test file to better test the results.

@cbrnr
Copy link
Owner

cbrnr commented Mar 28, 2017

@stfnrpplngr is this intended? Shouldn't there be at least some non-empty strings in the marker stream?

@cbrnr
Copy link
Owner

cbrnr commented Apr 25, 2017

@stfnrpplngr @Yida-Lin any progress on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants