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

Add FHIR format datalist for 3D classification example #32

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Sep 24, 2020

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.

Copy link
Contributor

@wyli wyli left a 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

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Sep 24, 2020

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
Copy link

A few comments;

  1. 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?
  2. 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)
  3. 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)
  4. 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://.
  5. 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.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Oct 16, 2020

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-16T19: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
Copy link

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.

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