-
Notifications
You must be signed in to change notification settings - Fork 765
Add FHIR format datalist for 3D classification example #32
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
Conversation
Signed-off-by: Nic Ma <nma@nvidia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @Nic-Ma would be nice to have a review from the data working group, I place a "request change" for now
thanks @Nic-Ma would be nice to have a review from the data working group, I place a "request change" for now
Sure, sounds good a plan.
Thanks.
IntegratorBrad
commented
Oct 6, 2020
A few comments;
- Did you run 3d_classification/torch/ixi_datalist.json through a tool like https://wiki.hl7.org/Using_the_FHIR_Validator to validate that it is valid JSON FHIR?
- Why did you choose "batch" instead of "collection" for the bundle type? Batch suggests that they need to be POST'ed / PUT on their own; collection might be a better fit. (https://www.hl7.org/fhir/bundle.html)
- I think the content type for the NIFTI is going to be application/x-gzip, not image/nifti. (http://hl7.org/fhir/datatypes.html#Attachment)
- The URL in the content attribute should look more like a URI. Looking at https://tools.ietf.org/html/rfc1738, it could start with file://.
- If you are bundling the Note as something significant (e.g., an Observation), that's probably not the ideal way to do it. I think in that case you would want a bundle of bundles; and in each bundle, you would have two objects - a Media NIFTI resource and an Observation resource. Perhaps even better, you could also use "partOf" in Media and reference a URI of something like Observation in the same Bundle collection.
Hi @IntegratorBrad ,
Sorry for my late response, we are busy with MONAI 0.4 tasks for stability.
Thanks very much for your review and I updated the PR today to address your comments 2, 3 and 4.
For comment 1, I tried to run with the tool but got some error:
$ java -jar org.hl7.fhir.publisher.jar tutorials/3d_classification/torch/ixi_datalist.json
Run time = Friday, October 16, 2020 at 7:06:43 PM Hong Kong Standard Time (2020年10月16日T19:06:43+08:00)
Package Cache: /home/nma/.fhir/packages (00:00.0031)
Load Configuration from tutorials/3d_classification/torch/ixi_datalist.json (00:00.0044)
Root directory: /home/nma/WorkSpace/Deep_Learning/Clara_SDK/medical/tutorials/3d_classification/torch (00:00.0067)
Publishing Content Failed: null (00:00.0069)
(00:00.0069)
Use -? to get command line help (00:00.0070)
(00:00.0070)
Stack Dump (for debugging): (00:00.0070)
java.lang.NullPointerException
at org.hl7.fhir.igtools.publisher.Publisher.initializeFromJson(Publisher.java:1949)
at org.hl7.fhir.igtools.publisher.Publisher.initialize(Publisher.java:1338)
at org.hl7.fhir.igtools.publisher.Publisher.execute(Publisher.java:701)
at org.hl7.fhir.igtools.publisher.Publisher.main(Publisher.java:7756)
And for comment 5, I didn't spend much time to study FHIR structures, just referred to your samples in the working group proposal. Could you please help understand what I should modify to address comment 5?
Thanks.
IntegratorBrad
commented
Oct 27, 2020
I posted a thread in FHIR Zulip chat to check with the community. Essentially though;
A challenge with a single object - e.g.,
{
"resource": {
"resourceType": "Media",
"id": "30",
"status": "unknown",
"type": "image",
"content": {
"contentType": "application/x-gzip",
"url": "file://IXI585-Guys-1130-T1.nii.gz"
},
"note": {
"text": "man"
}
}
}
.. is that, what if you want to have both the original NIFTI and the annotation NIFTI? Maybe, if it is two NIFTIs, you would reference it as follows:
{
"resource": {
"resourceType": "Bundle",
"id": "ixi-datalist-30",
"meta": {
"lastUpdated": "2020-09-25T01:00:00Z"
},
"type": "collection",
"entry": [{
"resourceType": "Media",
"id": "ixi-rawdata-30",
"status": "unknown",
"type": "image",
"content": {
"contentType": "application/x-gzip",
"url": "file://IXI585-Guys-1130-T1.nii.gz"
}
},
{
"resourceType": "Media",
"id": "ixi-annotation-30",
"status": "unknown",
"type": "annotation",
"content": {
"contentType": "application/x-gzip",
"url": "file://IXI585-Guys-1130-T1-annotation.nii.gz"
},
"note": {
"text": "man"
}
}
]
}
}
This would support if you have multiple annotations on the same dataset, too.
The problem though still is that the "annotation" is in the note, which makes it free text, which makes it hard to port into other languages or applications. It feels as though it should be a custom FHIR resource, like this:
{
"resource": {
"resourceType": "Bundle",
"id": "ixi-datalist-30",
"meta": {
"lastUpdated": "2020-09-25T01:00:00Z"
},
"type": "collection",
"entry": [{
"resourceType": "Media",
"id": "ixi-rawdata-30",
"status": "unknown",
"type": "image",
"content": {
"contentType": "application/x-gzip",
"url": "file://IXI585-Guys-1130-T1.nii.gz"
}
},
{
"resourceType": "Annotation",
"id": "ixi-annotation-30",
"status": "unknown",
"type": "annotation",
"content": {
"contentType": "application/x-gzip",
"url": "file://IXI585-Guys-1130-T1-annotation.nii.gz"
},
"annotationCode": [
{
"coding": [
{
"system": "http://snomed.info/sct",
"code": "357009",
"display": "Closed fracture of trapezoidal bone of wrist"
}
]
}
],
"note": {
"text": "man"
}
}
]
}
}
... but this should be explored further with the FHIR community.
This PR added datalist file in FHIR format for the PyTorch 3D classification example, which is the first step for the proposal from data working group.