[ENH] Add a statement addressing metadata conflicts #761

effigies · 2021-03-17T14:10:54Z

Closes #138.

yarikoptic

some wording tune up

src/02-common-principles.md

yarikoptic · 2021-03-17T14:53:36Z

src/02-common-principles.md

+If an exact representation of BIDS metadata is not possible in the format,
+then the imaging file metadata SHOULD be undefined
+or set to the closest possible approximation of the BIDS metadata.


Suggested change

If an exact representation of BIDS metadata is not possible in the format,

then the imaging file metadata SHOULD be undefined

or set to the closest possible approximation of the BIDS metadata.

If only possibly an exact representation of BIDS metadata is possible in the format,

then the data file metadata SHOULD be set to the closest possible approximation of the BIDS metadata.

I think would make it less ambiguous. I do not think we should allow for completely undefined in such cases. If "representation" is not possible at all -- there is nothing to talk about.

The situation I was considering here is that someone might prefer to zero out a value rather than have an approximate but incorrect value in the header. For instance, suppose you have a slice timing sequence that doesn't fit any defined NIFTI_SLICE_SEQ_* field. I'd rather set it NIFTI_SLICE_UNKNOWN than the closest approximation.

I suspect this type of case is going to be relatively common for binary formats.

I see. Yes, for NIFTI1_SLICE_ORDER indeed should be set to NIFTI_SLICE_UNKNOWN and thus indeed "undefined".

Majority of formats likely to not have nil or None or explicit "unknown" like in your example to specify the "undefined" value. So might be worth explicitly stating to set to "undefined" if explicit value is reserved for that (i.e. not 0 for some int or float field)? and also make it as a measure of the last resort while still keeping "SHOULD" on approximate values to make those files usable with generic tools?

What do you think of 679b8e0?

ping @yarikoptic

src/02-common-principles.md

yarikoptic · 2021-03-17T23:24:21Z

src/02-common-principles.md

+
+In cases of conflict, the BIDS metadata is considered authoritative.
+If BIDS metadata is defined,
+format-specific metadata MUST NOT conflict to the extent permitted by the format.


how poor validator would be able to check that "approximately" the same, or this MUST NOT will not be validated, and thus not enforced?

as much as I like consistency, it might be pragmatically prohibitive to achieve it. E.g. if metadata is contained within 1GB (or 1TB?) data file and then extracted and fixed up in metadata file (might be just a minor fix) -- should 1G/TB be re-uploaded?

With that in mind I just wonder if we should keep it as SHOULD NOT (warning if mismatch; not an error if it was MUST NOT) level?

I could go with SHOULD NOT. What about something like:

Generally, if BIDS metadata is defined, format-specific metadata SHOULD NOT conflict, to the extent permitted by the format. Specific metadata fields may impose a strict requirement.

I'm thinking here of RepetitionTime and pixdim[4] which MUST agree.

TL;DR: because it should be less of fields we allow to overload than of the fields we do not, may be we should keep original specification, but we would need to 1. come up with the annotation to when "conflict" is allowed (e.g. for some "description" fields etc) in side cars. 2. when allowed in metadata from filename entities (e.g. subject or session).

FWIW My longer thinking/line of argument

It seems then again to require per sidecar field "equality" specification that some could be allowed to disagree (thus it would be only a WARNING if sidecar has different value) whenever others MUST agree (and thus ERROR from validator). Most likely the strict requirements would then be added a explicit checks in the schema, like ATM checks being added for lengths consistency e.g. in https://github.com/bids-standard/bids-specification/blob/HEAD/src/schema/rules/checks/asl.yaml#L26 . That would indeed make it explicit but standard itself IMHO somewhat "inconsistent" since it would allow for some fields to be overloaded but not the others, and "by default" allow for them being different. Note that adding a restriction (from a default of "could different") in principle constitutes backward incompatible change, thus should be avoided.

In general though it is a tough aspect to allow such differences since it mandates that tools must now be able to read that "overloaded" metadata from sidecar files. That is why, ideally, we should really strive for consistency. And that is why e.g. we are working with NWB file format folks to allow for ".overwrite.json" file to accompany ".nwb" file so that metadata could be overloaded at the level of the file format, and thus allowing for "cheap" adjustment of metadata if so desired. For NIfTI, the primary workhorse of BIDS, since most of the tools would need correct base metadata such as pixdim, we better indeed enforce their consistency with side car files.

So, overall, I think we MUST enforce consistency and if so desired, relax only for some sidecar metadata fields AND strive to allow for metadata overload at the original file format whenever possible/necessary.

satra · 2021-03-17T23:26:47Z

src/02-common-principles.md

+JSON (see [Key/value files](#keyvalue-files-dictionaries)).
+In some cases, this duplicates metadata contained in a data file.
+
+In cases of conflict, the BIDS metadata is considered authoritative.


could we clarify what a conflict is? for example, is a change of floating point precision a conflict? one reason we store things in binary is because floating point data maintains some veracity. if we see this outside the nifti lens for other modalities allowed by bids (the eeg/meg or upcoming ephys/microscopy formats, where precision starts mattering).

could we clarify what a conflict is?

I can think of a few:

Equivalent fields, different values. RepetitionTime vs pixdim[4]

Disparate representations. PhaseEncodingDirection (enum) vs dim_info (bitfield)

Range of representations. PhaseEncodingDirection (axis + direction) vs dim_info (axis)

for example, is a change of floating point precision a conflict?

I'm interpreting this as a question of tolerance? Yes, there should be some tolerance. I imagine the order of tolerance would probably be a case-by-case question.

one reason we store things in binary is because floating point data maintains some veracity. if we see this outside the nifti lens for other modalities allowed by bids (the eeg/meg or upcoming ephys/microscopy formats, where precision starts mattering).

This seems to be arguing that we should say the highest-precision metadata should be authoritative, whether BIDS or format-internal. Or am I misunderstanding?

This seems to be arguing that we should say the highest-precision metadata should be authoritative, whether BIDS or format-internal.

in general yes, but we may want to clarify that for floating point values, the internal format always wins when close. this would perhaps avoid the situations where some has zeroed out something like TR.

This feels like a hard thing to write a rule for. Here's something:

In general, the more precise metadata representation SHOULD be considered authoritative, with BIDS metadata being presumptively more precise. A notable exception to this presumption is if a value is represented as a floating point value in the internal metadata format and a decimal string in the JSON sidecar; to avoid accumulating error in float-decimal conversions, the floating point value is authoritative.

This actually contravenes existing practice in fMRIPrep and FitLins with regard to RepetitionTime/pixdim[4], and may do so for MNE-Python as well, but I guess that's what standardization is for.

sounds good to me.

ping @satra

i like @effigies paragraph - but i think he still had the question out for the community.

@effigies please digest it for me a little on an example. E.g. if we have a value 0.3(3) (to the floating point precision) in the binary file and have 0.333 (to some arbitrary text precision) in sidecar file, the side car 0.333 would be taken as "authorative" but if we have it in side car as "0" or as "1", only then then the data file would have it as "authorative"?

@yarikoptic I intended to write the paragraph 4 comments up to mean (and still read it as saying) "floating point fields in a binary format take precedence over JSON, which is inherently decimal".

to freshen up: re my comment on floating point representation in text. data representation would also be truncated and it would just be a matter of expressing enough precision in json form.

E.g. specifically for my own example messing around in ipython

In [1]: repr(1/3) Out[1]: '0.3333333333333333' In [2]: 0.3333333333333333 * 3 Out[2]: 1.0 In [3]: 3.333333333333333e-1 * 3 Out[3]: 1.0 In [4]: 3.33333333333333e-1 * 3 Out[4]: 0.9999999999999989 In [5]: 1/3 == 3.333333333333333e-1 Out[5]: True In [6]: 1/3 == 3.33333333333333e-1 Out[6]: False

so may be we should just add somewhere wording that "serialization of floating point numbers in json should be to maximal precision of the underlying data type used in the data file" or something like that?

yarikoptic · 2022-07-12T17:53:02Z

src/02-common-principles.md

+then the closest possible approximation of the BIDS metadata SHOULD be used.
+Special null or undefined values MAY be used when available.
+If the format-specific metadata field is defined,
+the BIDS metadata SHOULD also be defined.


could someone give an example for the last one here?

If NIfTI slice_code != 0, then SliceTiming SHOULD be defined?

slice_code != 0 is not just that slice_code is defined but rather meets some criteria (!=0) and looking at https://nifti.nimh.nih.gov/nifti-1/documentation/nifti1fields/nifti1fields_pages/slice_code.html describing it as " slice_code = If this is nonzero, AND if slice_dim is nonzero, AND if slice_duration is positive,..." so it is even more complex than just defined. Moreover from above sentence it somehow suggests that it is just a matter of "mirroring" some value between file and BIDS metadata. But in this case it is not the case since SliceTiming is not even in NIfTI, which only specifies the order and timing is not contained in the file. So I will suggest this

effigies · 2022-07-13T21:07:53Z

I'm not sure what approving reviews are approving at this point. There is an open proposal to completely reverse the priority to have file metadata take precedence over BIDS metadata.

yarikoptic · 2022-07-13T21:41:54Z

src/02-common-principles.md

+If the format-specific metadata field is defined,
+the BIDS metadata SHOULD also be defined.


Suggested change

If the format-specific metadata field is defined,

the BIDS metadata SHOULD also be defined.

Some BIDS metadata can be REQUIRED to be defined depending on the values of the format-specific metadata fields.

although even then the slice_code target use case is quite hard to guess ;) but it also has nothing to do with the overloading really. May be should just be removed?

yarikoptic · 2022-07-13T21:46:50Z

There is an open proposal to completely reverse the priority to have file metadata take precedence over BIDS metadata.

yeah, and it seems that we are arriving that overall there might be no effective precedence and metadata must be in agreement between file metadata and BIDS metadata.

My approval was for the original suggestion, I'll withdraw it now and dive back into the discussion.

sappelhoff · 2022-10-21T08:48:52Z

migrating to 1.9.0 milestone because apparently this needs more discussion.

effigies · 2023-03-16T13:23:54Z

I've updated this PR to master because it's listed in the 1.9.0 milestone. I don't think there's a clear consensus on what to do. I've personally kind of given up hope on it, and would appreciate if a motivated party took over leading the discussion in a new issue/PR.

effigies mentioned this pull request Mar 17, 2021

Precedence in JSON/NIfTI metadata mismatch #138

Open

sappelhoff previously approved these changes Mar 17, 2021

View reviewed changes

yarikoptic requested changes Mar 17, 2021

View reviewed changes

yarikoptic reviewed Mar 17, 2021

View reviewed changes

satra reviewed Mar 17, 2021

View reviewed changes

sappelhoff mentioned this pull request Jun 7, 2021

MNE-BIDS Inspector modifies raw data – should we stop doing this? mne-tools/mne-bids#805

Closed

tsalo mentioned this pull request Jul 7, 2021

MRI: Add NumberOfVolumes? #828

Open

sappelhoff mentioned this pull request Jul 20, 2021

Clarifying anonymization for EDF files for EEG/iEEG #698

Closed

sappelhoff added this to the 1.7.1 milestone Jun 8, 2022

sappelhoff mentioned this pull request Jul 12, 2022

Question: consistency of metadata in .nwb and bids filename/side-car - must be consistent, or allowed to be overloaded? #1133

Open

yarikoptic reviewed Jul 12, 2022

View reviewed changes

robertoostenveld self-requested a review July 13, 2022 08:48

robertoostenveld approved these changes Jul 13, 2022

View reviewed changes

sappelhoff modified the milestones: 1.7.1, 1.8.0 Jul 13, 2022

sappelhoff changed the title ~~ENH: Add a statement addressing metadata conflicts~~ [ENH] Add a statement addressing metadata conflicts Jul 13, 2022

yarikoptic reviewed Jul 13, 2022

View reviewed changes

yarikoptic mentioned this pull request Jul 28, 2022

RF "files.py" validation/metadata-loading to support BIDS dandi/dandi-cli#1044

Closed

sappelhoff mentioned this pull request Aug 6, 2022

If no electrode positions are specified in the BIDS dataset metadata (electrodes.tsv), we should remove the montage upon loading the data mne-tools/mne-bids#1040

Open

sappelhoff modified the milestones: 1.8.0, 1.9.0 Oct 21, 2022

sappelhoff modified the milestones: 1.9.0, 1.8.0, 1.8.1 Oct 26, 2022

ftadel mentioned this pull request Nov 15, 2022

[BIDS] import NIRS dataset (fixes #469) brainstorm-tools/brainstorm3#589

Merged

sappelhoff modified the milestones: 1.8.1, 1.9.0 Feb 14, 2023

ENH: Add a statement addressing metadata conflicts

3a77356

effigies force-pushed the enh/metadata_statement branch from 077223e to 3a77356 Compare March 16, 2023 13:22

effigies requested a review from DimitriPapadopoulos as a code owner March 16, 2023 13:22

sappelhoff mentioned this pull request Apr 28, 2023

Sample NIRS data not read, problem with channel naming mne-tools/mne-bids#1117

Open

sappelhoff modified the milestones: 1.9.0, 1.10.0 Jun 29, 2023

sappelhoff mentioned this pull request Oct 24, 2023

Automatically assigning metadata mne-tools/mne-bids#1162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Add a statement addressing metadata conflicts #761

[ENH] Add a statement addressing metadata conflicts #761

effigies commented Mar 17, 2021 •

edited by yarikoptic

Loading

yarikoptic left a comment

yarikoptic Mar 17, 2021

effigies Mar 17, 2021

yarikoptic Mar 17, 2021

effigies Mar 17, 2021

sappelhoff May 22, 2021

yarikoptic Mar 17, 2021

effigies Mar 18, 2021

yarikoptic Jul 12, 2022

satra Mar 17, 2021

effigies Mar 18, 2021

satra Mar 18, 2021

effigies Mar 18, 2021

sappelhoff May 22, 2021

satra May 22, 2021

yarikoptic Jul 12, 2022

effigies Jul 13, 2022 •

edited

Loading

yarikoptic Jan 18, 2023

yarikoptic Jul 12, 2022

effigies Jul 12, 2022

yarikoptic Jul 13, 2022 •

edited

Loading

effigies commented Jul 13, 2022 •

edited

Loading

yarikoptic Jul 13, 2022

yarikoptic commented Jul 13, 2022

sappelhoff commented Oct 21, 2022

effigies commented Mar 16, 2023

		If the format-specific metadata field is defined,
		the BIDS metadata SHOULD also be defined.

	If the format-specific metadata field is defined,
	the BIDS metadata SHOULD also be defined.
	Some BIDS metadata can be REQUIRED to be defined depending on the values of the format-specific metadata fields.

[ENH] Add a statement addressing metadata conflicts #761

Are you sure you want to change the base?

[ENH] Add a statement addressing metadata conflicts #761

Conversation

effigies commented Mar 17, 2021 • edited by yarikoptic Loading

yarikoptic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effigies Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarikoptic Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

effigies commented Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

yarikoptic commented Jul 13, 2022

sappelhoff commented Oct 21, 2022

effigies commented Mar 16, 2023

effigies commented Mar 17, 2021 •

edited by yarikoptic

Loading

effigies Jul 13, 2022 •

edited

Loading

yarikoptic Jul 13, 2022 •

edited

Loading

effigies commented Jul 13, 2022 •

edited

Loading