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

fix(tmtv):Change volume unit from mm³ to cm³ in ROI thresholding display #4373

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JeffreyWW
Copy link

@JeffreyWW JeffreyWW commented Sep 14, 2024

Context

In the getLesionStats function of extensions/tmtv/src/commandsModule.ts, the volume is calculated as voxelCount * spacing[0] * spacing[1] * spacing[2] * 1e-3. This 1e-3 means that the final unit should be cubic centimeters rather than millimeters. Additionally, the final TMTV unit is in mL, which corresponds correctly.

Copy link

netlify bot commented Sep 14, 2024

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit 5a9e587
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/66ed3963552df80008bf3dfa

Copy link

netlify bot commented Sep 14, 2024

Deploy Preview for ohif-platform-docs canceled.

Name Link
🔨 Latest commit 5a9e587
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/66ed39635fb9a60008dddd96

@sedghi
Copy link
Member

sedghi commented Sep 19, 2024

@salimkanoun

@salimkanoun
Copy link
Contributor

mhhh I would prefer TMTV expressed in ml (which have been used in several publication and more standard unit to express volume in medicine)

@JeffreyWW
Copy link
Author

mhhh
test222
I agree with your point, but what I was addressing is not the unit of TMTV, but the unit for the segment’s volume. I noticed that in the calculation, it multiplies by 1e-3 at the end, so I changed it to cubic centimeters.

@salimkanoun
Copy link
Contributor

Yes sure but maybe it is better to remove the 10-3 rather than changing the unit

@JeffreyWW
Copy link
Author

Yes sure but maybe it is better to remove the 10-3 rather than changing the unit

Now I have done this. Thank you for your suggestion.

@salimkanoun
Copy link
Contributor

Thanks ! Sounds good !

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.

3 participants