spinalcordtoolbox / PAM50

https://github.com/neuropoly/spinalcordtoolbox
2 stars 1 forks source link

Add cauda equina label to make lumbar registration more user-friendly #26

Closed joshuacwnewton closed 1 year ago

joshuacwnewton commented 1 year ago

As part of https://github.com/spinalcordtoolbox/spinalcordtoolbox/issues/3599 and https://github.com/spinalcordtoolbox/spinalcordtoolbox/issues/4180, we have two good forum posts demonstrating how to perform lumbar registration with a new custom label.

In both cases, we asked the users to manually label a location on the PAM50 template representing the cauda equinea:

Some questions:

jcohenadad commented 1 year ago

Thank you for these excellent questions @joshuacwnewton

What coordinate should we use for this label? The origin label [70,69,46].

I think this is fine.

 The adjusted label [70,70,40].

I think this is also fine. Probably better in fact, because as you pointed out the new cord seg should now be available in PAM50 (or will be at the time of the course)

Re-use label 21 [70,70,39]

I would not do that, as it might create confusing with the existing label, users will forget to put it back afterwards, etc. I can see a lot of issues that would be hard for us to debug remotely

What value should we use for this label? In the forum posts, a value of 99 is used for the cauda equinea label. However, this label is (kind of) reserved for the dummy label used in sct_register_to_template, plus this value doesn't really fit with the existing label conventions (1-25, 49, 50).

Yeah I agree. How about we go with 90+. Like 90, 91, 92, etc.

So, perhaps we should use something like 30 or 40? (This would follow the example given by the PMJ-related labels 50/49, while also giving plenty of room above and below for future labels, since the L5-S1 disc uses label 25.)

I'd rather keep the 1-->70ish for us. Putting custom values "in between" might create confusion.

joshuacwnewton commented 1 year ago

Yeah I agree. How about we go with 90+. Like 90, 91, 92, etc.

I'm very okay with this. :)

I'd rather keep the 1-->70ish for us.

Just to clarify, though, what do you mean by "for us"?

In other words, I'm curious how the cauda equinea label is any different from, e.g. the PMJ label (50). (Is there a specific rationale for why the value 50 was acceptable for PMJ, but why 1-70 is unacceptable for the cauda equinea?)

joshuacwnewton commented 1 year ago

Ah wait, my apologies. PAM50_label_disc.nii.gz does not actually have the labels 49,50 on it! (I thought it did thanks to the Labeling conventions page.)

In that case, using label 90 makes a lot of sense. :)

jcohenadad commented 1 year ago

Just to clarify, though, what do you mean by "for us"?

i meant values of labels included in the PAM50 or future templates (eg the work of Nilser)

jcohenadad commented 1 year ago

@joshuacwnewton I'm very sorry but I misunderstood what was the intention of this issue and related PR-- I thought the plan was only to create a tutorial where we show how to make custom labels (hence the suggestion of 90+ values), but in fact the proposal is to modify the "official" labels by adding one. Hence this falls into the category of "our labels" (that's what I meant here https://github.com/spinalcordtoolbox/PAM50/issues/26#issuecomment-1744968880), and therefore I would use another value than 90 (reserving 90+ for users who would like to create new labels).

Your solution (ie: modify our PAM50_label_disc.nii.gz) is in fact a better solution, because it prevents the dirty hack of asking users to modify source PAM50 files. But in this case, I would propose to use another value, like 60 maybe?

joshuacwnewton commented 1 year ago

but in fact the proposal is to modify the "official" labels by adding one.

Exactly. (My plan was to address both https://github.com/spinalcordtoolbox/spinalcordtoolbox/issues/4180 and https://github.com/spinalcordtoolbox/spinalcordtoolbox/issues/3599 in one go.) My apologies if my intention was unclear. :sweat_smile:

But in this case, I would propose to use another value, like 60 maybe?

Exactly my thinking! I'll update the PR to use this value instead.