hed-standard / hed-javascript

HED/BIDS-friendly JavaScript validator.
https://hed-javascript.readthedocs.io/en/latest
MIT License
3 stars 7 forks source link

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

Closed sappelhoff closed 4 weeks ago

sappelhoff commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

This is essentially issue #141

happy5214 commented 4 weeks ago

Closed as duplicate of #141.