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

HED string passes validation with JS/BIDS HED validator, but not online string validator #168

Closed
sappelhoff opened this issue Aug 9, 2024 · 3 comments
Labels
duplicate This issue or pull request already exists
Milestone

Comments

@sappelhoff
Copy link
Member

In the eeg_matchingpennies example on the BIDS-examples repository, we have the following HED string:

https://github.com/bids-standard/bids-examples/blob/4460013876560adf95ecef44dc59a5883d24a30b/eeg_matchingpennies/task-matchingpennies_events.json#L58

It passes the validation of the bids-validator, which means it also passes HED validation.

However, passing this string into the online string validator at https://hedtools.org/hed/strings yields the following errors:

Errors for HED string 0:
TAG_REQUIRES_CHILD: Descendant tag required - 'Delay'
TAG_REQUIRES_CHILD: Descendant tag required - 'Duration'
CHARACTER_INVALID: Invalid character ' ' in tag 'Time-block/68 ms'

String used:

(Agent-action, Participant-response, (Lift, (Hand, (Left-side-of, Experiment-participant))), (Release, (Mouse-button, (Left-side-of, Experiment-participant)))), (Delay, Duration, (Approximately-equal-to, Time-block/68 ms)), (Sensory-event, Visual-presentation, (Drawing, (ID/left_hand.png, Hand, (Left-side-of, Computer-screen))), (Feedback, Penalty)), Description/Subject raised left hand and computer presented image of hand to left side

The errors can be solved by taking this part:

(Delay, Duration, (Approximately-equal-to, Time-block/68 ms))

and reducing it to:

Delay/#68ms

... although I feel like the nuance of that 68ms is only "approximated" is lost.

@VisLab
Copy link
Member

VisLab commented Aug 9, 2024

The hed-javascript validator does not yet fully validate the handling of Duration --- this was specified relatively recently and we are working on it now. This string should not validate if you are using HED schema version 8.3.0.

The Delay and Duration tags now have the requireChild attribute (starting with HED version 8.3.0). This is needed because if people specify more than one event in a single line of the event file (e.g., a line represents a trial with say a stimulus presentation and a response time) we wanted to be able to represent the actual time of the delayed event -- in this example the response).

In (Delay, Duration, (Approximately-equal-to, Time-block/68 ms)) there is no way to calculate when the event representing this annotation occurred.

I realize that HED doesn't not currently have a good annotation mechanism for representing that something is an approximate alue.

However, in looking at Delay, Duration, (Approximately-equal-to, Time-block/68 ms)), I wouldn't know what was being delayed and what duration was being expressed.

(Delay/10 ms, Duration/68 ms, (xxx)) indicates xxx started 10 ms after the onset time on the line it appears and lasted for 68 ms.

The specification discussion is: Temporal scope

This was specified in the latest spec so that tools could correctly unfold events.

@VisLab
Copy link
Member

VisLab commented Aug 30, 2024

This is essentially issue #141

@happy5214
Copy link
Member

Closed as duplicate of #141.

@happy5214 happy5214 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 11, 2024
@happy5214 happy5214 added the duplicate This issue or pull request already exists label Oct 11, 2024
@happy5214 happy5214 added this to the 3.16.0 milestone Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants