Niffler icon indicating copy to clipboard operation
Niffler copied to clipboard

Better configurations for Destination AE to be different from Query AE

Open pradeeban opened this issue 3 years ago • 1 comments

Niffler will need some modification that when we use an external AE to store the images, storescp does not need to be stared locally. I will create a bug report for that. But it shouldn't cause any trouble. It is just Niffler also creating a storescp unnecessarily for an iteration where we just want to send the data to another AE's storescp.

A solution will be, a system.json parameter, "IsExternalAE" and if it is true, not to start the local storescp process.

pradeeban avatar May 25 '21 19:05 pradeeban

I am working on this issue.

karthik235689 avatar Mar 17 '22 10:03 karthik235689

hey @pradeeban I am a complete newbie but I would like to work on this issue. from what I understand we need to add a new setting to Niffler called "IsExternalAE" which would tell Niffler not to start the storescp process unnecessarily. could you help me out with more context/guidance, Thanks :)

yuvraj-wale avatar Feb 25 '23 07:02 yuvraj-wale

@yuvraj-wale That is correct.

We specify these Query AET and Destination AET through system.json

"QueryAet": "QBNIFFLER:4243",
"DestAet": "QBNIFFLER",

Usually, the QueryAet = DestAet in our configurations (as shown above). However, we had made the assumption that it will be the case always when we developed Niffler. That is an incorrect assumption, but we did not bother fixing it (since we did not encounter the below scenario.

"QueryAet": "QBNIFFLER:4243",
"DestAet": "DTNIFFLER",

Above, DTNIFFLER port and hostname need not be defined in the system.json, since those are defined in the Src AET (PACS or Scanner) configurations anyway.

So, as you interpreted it, a field such as "isExternalAE" to check whether a local storescp needs to be started can be an option.

But a better way would be, to parse QueryAet and DestAet to see whether they are the same. In case 1, they are the same. Both are QBNIFFLER. So, you must start the storescp there.

In case 2, QBNIFFLER is not the same as DTNIFFLER. That means, you should not start the storescp in QBNIFFLER (QueryAet).

Now I recommend this approach as it avoids having to introduce a new parameter which will confuse the users further. This way, it is just a text-parsing.

Let's say the current code in pseudocode:

START-STORE-SCP

The suggested code change:

BREAK QUERY AET BY ":" TO FIND THE QUERY AET's AET VALUE.
IF QUERY AET's AET VALUE === DEST AET
	START-STORE-SCP

This assumes that Dest Aet takes care of the storescp process separately (outside Niffler, as it is an "ExternalAE" as we interpret). But eventually/ideally, we can include a script (a smaller version of cold-extraction module) to manage the StoreSCP process there as well.

Please feel free to get back to me with more questions on this.

pradeeban avatar Feb 25 '23 14:02 pradeeban

@pradeeban I'm interested in this one. Actually I'd like to know how and where is the STORESCP implemented. Also I'd like to ask if there is a Documentation/resource to understand the overall architecture of Niffler.

rumbleFTW avatar Feb 25 '23 15:02 rumbleFTW

@rumbleFTW Search for "storescp" in https://github.com/Emory-HITI/Niffler/blob/master/modules/cold-extraction/ColdDataRetriever.py and follow through.

The main README and the README in each module are good starting points. There are also research papers linked in the main README if you like to learn more from the higher level.

Of course, pls go through the source code and understand it first. Often a rushed PR for an open-source project repository ends up with bugs. We are in no rush to fix this.

pradeeban avatar Feb 25 '23 15:02 pradeeban

@pradeeban Thanks! Yeah you are right, I'm actually studying the papers associated to the projects first in order to get a clear idea of how all of these work.

rumbleFTW avatar Feb 25 '23 15:02 rumbleFTW

Hello @pradeeban, I read your comments and made modifications to the code. Could you help me out on how I could test the code?

yuvraj-wale avatar Feb 25 '23 16:02 yuvraj-wale

hey @rumbleFTW I see you are interested in this issue as well, actually I had started working on this one quite some time ago and made some significant progress, just mentioning to avoid duplicate effort 😅

yuvraj-wale avatar Feb 25 '23 17:02 yuvraj-wale

@yuvraj-wale Duplicate efforts are fine at this point as we are at the GSoC and PRs are mostly for learning the code (and highlighting your effort) at this point.

pradeeban avatar Feb 25 '23 17:02 pradeeban

@yuvraj-wale Testing this one is a challenge on its own. You will need 3 AETs.

  • One is a PACS or an Orthanc deployment with DICOM images (perhaps, taken from The Cancer Image Archive).
  • Second one is where Niffler QueryAET is running.
  • Third one is another instance of a storescp such as a DCM4CHE.

Since that is quite an effort, if you just send a PR, I can go through it first to see if you are in the right path and propose changes (or merge, if you are really lucky to get it right at your first attempt).

pradeeban avatar Feb 25 '23 17:02 pradeeban

CC/ @anbhimi, you mentioned you and your GSoC 2022 student made an article on deploying Orthanc for testing purposes. Can you please link it here for future reference?

I think such a document should be integrated and/or highlighted earlier in the main README.md of the cold-extraction module as well.

pradeeban avatar Feb 25 '23 17:02 pradeeban

@yuvraj-wale Sure. You can carry-on your work in this issue. I'll be looking over another issue for now. Glad you let me know beforehand ^_^

rumbleFTW avatar Feb 25 '23 17:02 rumbleFTW

Hey @pradeeban, made the PR check it out #376.

yuvraj-wale avatar Feb 25 '23 18:02 yuvraj-wale

Hi, We have included the article in the cold extraction module's README.md a while ago. Attaching the link below -

https://github.com/Emory-HITI/Niffler/tree/master/modules/cold-extraction

anbhimi avatar Feb 26 '23 03:02 anbhimi

Thanks a lot, @anbhimi, for the documentation!

pradeeban avatar Feb 26 '23 03:02 pradeeban

Although not tested for functionality as addressed in #378, the code is merged to dev.

pradeeban avatar Mar 03 '23 17:03 pradeeban