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.

Sign up for GitHub

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

Jump to bottom

BEP for dimensionality reduction-based networks #1378

Open
PeerHerholz opened this issue Jan 10, 2023 · 42 comments
Open

BEP for dimensionality reduction-based networks #1378

PeerHerholz opened this issue Jan 10, 2023 · 42 comments
Labels
BEP derivatives

Comments

@PeerHerholz
Copy link
Member

Your idea

Hi @bids-maintenance & everyone,

I hope you're doing fine.

This issue is meant to track/gauge interest and progress for a specification focusing on dimensionality reduction-based networks, e.g. functional networks based on ICA/PCA, structural networks based on covariance, etc. . It originated as part of the BIDS connectivity extension(s) project during the project's first workshop last September. Given its intended scope we aim to collaborate with the majority of the modality-specific, model, as well as other connectivity BEP teams.

The team is currently working on a draft here and we plan to have a meeting discussing the draft at the end of January (I'll update here once we have a date & time).

It would be cool to get all your ideas and feedback on this!

Cheers, Peer
@sappelhoff sappelhoff added the BEP label Jan 10, 2023
@PeerHerholz
Copy link
Member Author

Hi @bids-standard/maintainers,

as we aim to share our draft with the community to receive and integrate feedback soon, we would like to ask if it would be possible to move our BEP to the next stage. More specifically, this refers to step 5 of the BEP development guide, ie making the BEP official and obtaining a number. We followed and conducted steps 1-3 and would now like to access if the requirements for step 4 were fulfilled and if not, what would be missing to achieve this.

If you have any questions, please don't hesitate to ask.
We're looking forward to working with you on this.

Best, Peer

@PeerHerholz
Copy link
Member Author

Hi everyone,

I just wanted to follow up on my earlier message: would it be possible to evaluate if the requirements for step 4 were fulfilled and if not, indicate what we would need to do to achieve this?

It would be great to hear from you.

Best, Peer

@sappelhoff
Copy link
Member

Hi @PeerHerholz, thanks for the patience -- I am not sure I'm the correct person to judge this, but to obtain a number you may open a PR to the bids-website repo by editing these files. We can then discuss in that PR and potentially approve and merge it, and then you'll have the official number :-)

@PeerHerholz
Copy link
Member Author

Hi @sappelhoff,

thanks for getting back to us and the information. We opened a respective PR.

Looking forward to the discussion!

Cheers, Peer

@PeerHerholz
Copy link
Member Author

PeerHerholz commented Aug 30, 2023 •
edited
Loading

Hello @bids-standard/maintainers, @bids-standard/steering & everyone,

I hope you're doing fine.

We (@tincala91, @DrCyPhi, @anibalsolon, @dorahermes, @adelavega, @rwblair, @francopestilli and others) continued to work on BEP039 - Dimensionality reduction-based networks. We would like to finalize Step 9 of the BEP development process: Incorporate the feedback and strive for consensus. . Thus, I thought about creating a dedicated issue within which we can track what is still needed in order to do so.

If y'all could have a look at the draft again and share your thoughts re the current status and if something/what definitely still needs to be addressed, that would be great! I'll start a list below and will keep editing it based on your comments.

Needs to be addressed

  • add single subject EEG ICA example
  • clean up EEG related paragraphs

Would be cool to address but not necessary

Thanks so much again for all your help and effort, we highly appreciated it.

@DrCyPhi
Copy link

DrCyPhi commented Aug 30, 2023 via email

@PeerHerholz
Copy link
Member Author

Hi @DrCyPhi,

of course, no worries at all.

@dorahermes
Copy link
Member

My main concern is that it does not capture a highly common use case of ICA on individual subject EEG data. It would be valuable if someone who has a lot of experience with EEG/ICA would have a look at it.

@DrCyPhi
Copy link

DrCyPhi commented Sep 4, 2023 via email

@francopestilli
Copy link
Collaborator

@DrCyPhi Thank you for doing this.

@PeerHerholz
Copy link
Member Author

Hi everyone,

thx @DrCyPhi and @dorahermes, I'll add it to the list!

@dorahermes
Copy link
Member

Thank you @DrCyPhi !

@PeerHerholz
Copy link
Member Author

Hi folks,

bumping this up again. What do you think re the current status of the draft?
Should we schedule a meeting early next year to discuss and start the next steps?

Cheers, Peer

@PeerHerholz PeerHerholz mentioned this issue Jan 11, 2024
Open
4 tasks
@PeerHerholz
Copy link
Member Author

Hi folks,

I hope you're doing fine.

@francopestilli, @arokem, @dorahermes, @DrCyPhi, @CPernet, @tincala91, @bids-standard/maintainers & @bids-standard/steering (& everyone of course): I created a poll covering the next 2 weeks to find time for a 1h meeting to discuss the current state and future steps. You can find it here.

Thanks again.

Cheers, Peer

@francopestilli
Copy link
Collaborator

@PeerHerholz thank you for setting this up!

@PeerHerholz
Copy link
Member Author

Hi everyone,

thanks so much for taking the time to vote.

Based on the responses, it seems that next Tuesday, January 23, 5 PM CET, 10 AM CT, 8 AM PST works best.
I sent everyone who voted (@dorahermes, @tincala91) an invite. If others want to join as well (@DrCyPhi can you maybe make it?), please comment here and I can add you. You can find the agenda here. Please feel free to add items you want to discuss.

Thank you very much again. We're looking forward to the meeting!

Cheers, Peer

@PeerHerholz
Copy link
Member Author

Hi everyone,

thanks a lot for a very productive meeting yesterday.

Based on the things we discussed, we would like to ask @bids-standard/maintainers and @bids-standard/steering if we could schedule a meeting with you to talk about the current state and next steps. Specifically, this refers to moving on to the next stage, ie porting the draft to GitHub and adding examples.

I created a survey here to hopefully find a feasible time, covering the next 2 weeks.

It would be cool to hear from and meet with you.

Thanks again.

Cheers, Peer

@arokem
Copy link
Collaborator

arokem commented Jan 24, 2024

Could we please schedule this to occur at an already-scheduled upcoming meeting of the steering group? We have one coming up on February 1st at 6 AM PT / 1500 UTC, which is within the time-frame you indicated. Does that time/day work for you, @PeerHerholz? @kimberlylray: do we have anything scheduled for this meeting? Or could we get this item onto the agenda for the upcoming meeting?

@PeerHerholz
Copy link
Member Author

PeerHerholz commented Jan 24, 2024 •
edited
Loading

Hi @arokem,

thanks for the info! Yes, as far as I can tell now, this should work for me. @DrCyPhi and @tincala91: would you be able to join?

@tincala91
Copy link

tincala91 commented Jan 24, 2024 via email

@PeerHerholz
Copy link
Member Author

Hi @arokem and @kimberlylray,

I hope you're doing fine.

I wanted to ask if we could maybe briefly present the BEP during the next steering group meeting?

Thanks.

Cheers, Peer

@kimberlylray
Copy link

kimberlylray commented Feb 5, 2024 via email •
edited by tsalo
Loading

@PeerHerholz
Copy link
Member Author

Hi @cmaumet,

I was wondering how we should continue the discussion around parameters and provenance?
Should we draft something in the GoogleDoc?

Thanks again.

Cheers, Peer

@CPernet
Copy link
Collaborator

CPernet commented Feb 26, 2024 •
edited
Loading

following BIDS derivatives principles, I'd recommend having 2 approaches

  • the lightweight descriptions.tsv that gives the parameter space but not provenance
  • the BEP prov allowing to actually trace back exactly what was done

we tried to do that for eeg see https://docs.google.com/document/d/1PmcVs7vg7Th-cGC-UrX8rAhKUHIzOI-uIOh69_mvdlw/edit#heading=h.ruoibwsnpivw

@PeerHerholz
Copy link
Member Author

Thanks. Just to reiterate: we would get rid of the "model".tsv and "model".json in favor of the descriptions.tsv that would entail the information we had in the prior as columns and rows, correct?

@CPernet
Copy link
Collaborator

CPernet commented Feb 27, 2024

yes that's the idea - but it has to be tested if that works for you.

@PeerHerholz
Copy link
Member Author

Hi everyone (@dorahermes, @DrCyPhi, @tincala91, @CPernet and of course everyone else),

as we want to finalize this BEP asap, we would like to get a few roadblocks out of the way.
Specifically, this refers to the "model" and "index" keys we have to find different names for, as both are convoluted and/or misleading/already taken. Could I maybe ask y'all if you have some ideas concerning this?

Thanks!

Cheers, Peer

@dorahermes
Copy link
Member

Can you perhaps provide a more detailed description about what which terms 'model' and 'index' are intended to capture? (Will help for brainstorming, thanks!)

@PeerHerholz
Copy link
Member Author

Hi Dora,

yes, sorry for not being more precise:

We initially used "model" as a key to indicate what kind of model was applied/run via a respective ID. For example, if you ran an ICA, it would've been model-ICA. However, as the term "model" is currently heavily discussed and mostly its usage discouraged, we should find something akin that would allow us to define what was applied/run on the data. IIRC, at some point "result" was discussed (@effigies, sorry brashly tagging you).

We initially used "index" as a key to indicate which component a file refers to in case files are saved not in 4D. For example, if you ran an ICA and the outputs are stored in 3D files, it would've been model-ICA_index-1. However, as index is already used for something else in BIDS (e.g. here), we should find something akin that would allow us to define which component a file refers to.

HTH and sorry again.

Best, Peer

@effigies
Copy link
Collaborator

effigies commented May 3, 2024

I was not under the impression that model was discouraged, just its use as a directory for hierarchical organization (#1280 (comment)). To my mind, the main thing that would be potentially confusing for that entity is that you might have a case where you want to specify the model type (e.g., model-ICA, model-GLM, etc.) and another where all of your models are the same type and you want to give them specific names, (e.g. model-randomFx model-fixedFx). I don't think that kills the idea, just something to take into account.

I do think index-<index> is a bit problematic, since it is already a term we use for entity values, so doubling it up to also be an entity key is likely to lead to imprecision or to a lot of text to avoid ambiguity. Given that we have entities (echo, run, chunk) where the value is an index, if what we want to say is the 4th component, then something like component-4 would make sense to me. (We could potentially have a short form for component, like cmp-4). If you want something extremely generic, perhaps item-<index> could be defined to be something like "The index of an item in a collection of related files. The specific meaning of the index varies by context."

@arokem arokem mentioned this issue May 3, 2024
Open
@PeerHerholz
Copy link
Member Author

Hi @effigies,

thanks a lot for adding your feedback and ideas, that's highly appreciated.

I was not under the impression that model was discouraged, just its use as a directory for hierarchical organization (#1280 (comment)). To my mind, the main thing that would be potentially confusing for that entity is that you might have a case where you want to specify the model type (e.g., model-ICA, model-GLM, etc.) and another where all of your models are the same type and you want to give them specific names, (e.g. model-randomFx model-fixedFx). I don't think that kills the idea, just something to take into account.

Thanks. Ah, I see. The way I understood it was the model is generally problematic regardless of used as directory or entity name, as it's definition is rather complicated and convoluted. I don't have anything against atm but just wanted to address what folks discussed before. Re naming models: Yeah, we also discussed this and were wondering if some terms should be encouraged, ie a set of defined model types and their abbreviation/ID, or if folks should be allowed a bit more freedom as the model abbreviation/ID should be further explained in the corresponding metadata. The former option would be related to discussions in BIDS stats models and fitlins I think and at least for some models be possible.

I do think index- is a bit problematic, since it is already a term we use for entity values, so doubling it up to also be an entity key is likely to lead to imprecision or to a lot of text to avoid ambiguity. Given that we have entities (echo, run, chunk) where the value is an index, if what we want to say is the 4th component, then something like component-4 would make sense to me. (We could potentially have a short form for component, like cmp-4). If you want something extremely generic, perhaps item- could be defined to be something like "The index of an item in a collection of related files. The specific meaning of the index varies by context."

Thanks! That's what I thought. I think item-<index> would be a feasible option forward as it would general enough to also be useful for other things in BIDS as outlined by @arokem. WDYT @dorahermes, @DrCyPhi, @tincala91, @CPernet and of course everyone else?

Thanks again.

Cheers, Peer

@CPernet
Copy link
Collaborator

CPernet commented May 5, 2024

Given @effigies and @arokem comments, it seems that model- is fine. As a side note, it also means that within BIDS this will have a very general definition as an entity that refers to an implicit (ICA) or explicit (ballstricks, glm) representation of the data (over any dimension).

@PeerHerholz In the BEP, index- indicates which component it is in the ICA model. Are there other cases for which `index—' is used? (for models, not entities). I cannot see other instances in the examples. thx

@DrCyPhi
Copy link

DrCyPhi commented May 5, 2024 via email

@PeerHerholz
Copy link
Member Author

Hi @CPernet and @DrCyPhi (& everyone),

thanks for your feedback!

Cool, it seems that model as an entity is ok for folks. We will then keep it for now.

Re the index entity: as outlined by @DrCyPhi, we so far only used it for components not saved in 4D files.
Great, item seems to be a feasible way forward.

I'll update the GoogleDoc respectively to see "how it looks". The prior version will, of course, be saved via
version control.

Thanks so much again.

Cheers, Peer

@PeerHerholz
Copy link
Member Author

Hi @bids-standard/maintainers,

after talking with @effigies during the Brainhack, I wanted to ask if it be possible to have a somewhat formal review/assessment of the current state of the GoogleDoc draft, so that we can then hopefully move things over to GitHub?

It would be cool to hear from you.
Thanks.

Best, Peer

@effigies
Copy link
Collaborator

effigies commented Jul 25, 2024 •
edited
Loading

@PeerHerholz Would you be available at 1pm EDT on August 8 to have a call with maintainers and go over the state of the docs? (Answering here for BEP17 as well.)

@PeerHerholz
Copy link
Member Author

Yeah, that should work. Thanks a lot @effigies.

@PeerHerholz
Copy link
Member Author

Hi @effigies,

I just wanted to follow re a meeting for this BEP, as we didn't get to discuss it last time.

Cheers, Peer

@effigies
Copy link
Collaborator

Right. @bids-standard/maintainers do we have time next week or should we schedule separately?

@PeerHerholz
Copy link
Member Author

Hi everyone,

we just had our meeting and I wanted to provide a quick update here:

  • overall, the BEP is in good shape and we can move to the next stage, ie putting things in PRs
  • we will divide the work into three distinct yet complimentary efforts:
    • the item- entity will be discussed and introduced via its own PR
    • the group- entity will be discussed and introduced via its own PR
    • the BEP will be initially discussed and introduced without the above entities
    • once the entities are added, the BEP aspects will be updated respectively via a PR

After addressing a few comments, the corresponding PRs will be opened and mentioned here.
That should be what we've discussed (@effigies, @nellh, @rwblair, @tsalo), right?

Please let us know if you have any questions.

Best, Peer

@tincala91
Copy link

tincala91 commented Oct 15, 2024 via email

@DrCyPhi
Copy link

DrCyPhi commented Oct 15, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BEP derivatives
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests