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

Current state of the BEP (NOT FOR MERGING) #24

Draft
wants to merge 87 commits into
base: master
Choose a base branch
from
Draft

Conversation

Lestropie
Copy link
Collaborator

This is a replacement of #6 as the BEP is being reinvested in.

The PR is not for merging, but to show the current state of the difference between the BEP and an up-to-date master of the BIDS specification.

effigies and others added 19 commits September 18, 2019 10:13
This PR splits the tractography section from the diffusion derivatives
document, so that #5 is easier to merge.
The new ``05-diffusion-derivatives-tractography.md`` file will remain
orphaned, but kept there as a base for the time we tackle tractography.
It shouldn't be merged into the derivatives branch until it is ready.
- More clarity of distinction between requisite and optional files in output directory.
- Try using 3 spaces rather than 4 in non-code indentation; partly to try to get tables within dot point lists to render correctly, partly to improve editor software syntax highlighting.
- Various small re-wordings.
- Slightly more use of hyperlinks.
- Short introductions to "parameter terminology" and "data representations" sections.
- Be more explicit about normalised vs. non-normalised 3-vectors, so that structure more clusely mimics that of description of spherical coordinate representation.
- Rename hyperlink names to "parameter terminology" section to better separate from later "intrinsic / extrinsic model parameters" sections.
Based on suggestion in #5. If all model intrinsic parameters are incorporated into a single file, rather than dropping the "_parameter-<param>" field, instead enforce that parameter name "all" be used.
When introducing the file naming convention, give an example of the "_<model>" field.
Re-arranged descriptions of intrinsic and extrinsic model parameters within the file naming section, and corrected a discordance in one dot point that was using an intrinsic model parameter filename path but discussing extrinsic model parameters.
Provide information on specification of orientation data after the various models and model parameters have been explained.
Lestropie and others added 9 commits September 13, 2022 15:54
In order to better disambiguate the various types of "parameters" defined, change "model parameters" to "model fit parameters".
BEP016: Adopt "model fit parameters" terminology
BEP016: Remove references to removed "pdf" data representation
Specification requests field "ModelDescription", but exemplars at end of document instead erroneously used key "Model".
DWI derivatives: Fix ModelDescription in exemplars
This commit/PR updates the bids-BEP016 repository to incorporate the changes/current state in the bids-specification repo.
synch bids-bep016 with current state of bids-specification [MISC]
@arokem
Copy link
Collaborator

arokem commented Nov 14, 2022

Looks like there are still a lot of unrelated things even after merging #78 in here.

@arokem
Copy link
Collaborator

arokem commented Nov 14, 2022

Ok - now merged with the specification master branch, this is (for now) the actual diff against the specification.

<pipeline_name>/
sub-<participant_label>/
dwi/
<source_keywords>[_space-<space>]_model-<label>_param-<param1>_model.nii[.gz]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this suffix to _mfp, along the lines of the ongoing work on dimensionality reduction networks?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change was agreed on the dimensionality reduction BEP after we last discussed with @Lestropie so Rob did know. Let's change it here too. Happy to get started when I back or Peer perhaps can?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not followed discussions elsewhere RE file suffices; too overloaded. If MFP / MDP may catch on more broadly, can crossref to #69 / #68 / #50 and start a dedicated PR.

- Mapping between image volume *V* and spherical harmonic basis
function coefficient *Y<sub>l,m</sub>*:

*V<sub>l,m</sub>* = (*l*(*l*+1) / 2) + *m*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arokem needs to write this for the Descoteaux SH basis as well

@arokem
Copy link
Collaborator

arokem commented Jun 28, 2023

Updated with current master branch on the upstream fork of the spec.

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

Successfully merging this pull request may close these issues.

None yet

6 participants