Git Product home page Git Product logo

Comments (10)

neurolabusc avatar neurolabusc commented on August 14, 2024

@hansenms any reason this issue was closed without fixing
IsmrmrdParameterMap_Siemens_EPI_FLASHREF.xsl and IsmrmrdParameterMap_Siemens_T1Mapping_SASHA.xsl?

from siemens_to_ismrmrd.

hansenms avatar hansenms commented on August 14, 2024

It has been open since 2017 with no repro and no suggested fix. If this is an issue we need a proper repro. Like an example dataset and a clear description of what your expected behavior is and what exactly you are seeing. Ideally there would be a PR to fix it.

So I mostly closed it because I figured if it had been sitting since 2017 without a PR it was probably no longer relevant. I would propose a PR instead of an issue if you have a fix.

from siemens_to_ismrmrd.

neurolabusc avatar neurolabusc commented on August 14, 2024

Understood. Can I suggest you keep this issue closed but assign the label wontfix. I did provide a link to a repository that provides sample images with different partial Fourier settings. However, it is completely understandable that addressing this is outside of the remit of the current developers. Both dcm2niix and GDCM provide correct solutions for any future users that need to accurately determine pF.

from siemens_to_ismrmrd.

hansenms avatar hansenms commented on August 14, 2024

I wouldn’t say it’s a won’t fix. Somebody just needs to take it on. There is a tendency that a lot of issues get opened with an expectation that some army of devs jump on and solve it. The reality is that you are probably in the best position to fix this and would have the test cases etc. Also in this case, the PR would probably be less text than this discussion. So if somebody would like a style sheet changes, let’s just get a PR.

from siemens_to_ismrmrd.

hansenms avatar hansenms commented on August 14, 2024

Actually, I am reopening this. It is not 100% clear these values are to be interpreted the same for all sequences and it may be intentional that they are the way that they are. However, we need to have a few more eyes on it. So I will put up a PR (#89) and have somebody else take a look at it.

In the future, it is a huge help if you just put up a PR for this sort of thing. Much quicker to move it through. Issues that require somebody else (who is not focused on it and has no tests cases) tend to just sit there.

from siemens_to_ismrmrd.

neurolabusc avatar neurolabusc commented on August 14, 2024

@hansenms I do believe that ucPhasePartialFourier is described similarly across all Siemens sequences - I have never heard of any errors from dcm2niix users. However, I would be happy to acquire some public validation datasets on our VE11C Siemens Prisma if someone can send me a sample of the EPI_FLASHREF or T1Mapping_SASHA DICOMs that I can Phoenix onto our system (send to the email listed in my avatar). I do host a number of public DICOM validation datasets dcm_qa* that might be useful for testing the siemens_to_ismrmrd project. I have also sent an email to our Siemens Research Collaboration Manager to verify that ucPhasePartialFourier has a single definition across all sequences.

from siemens_to_ismrmrd.

hansenms avatar hansenms commented on August 14, 2024

Let's see if @kspaceKelvin can get us the information. It would not be the first time that a parameter is interpreted differently for different sequences. For phase resolution some sequences round up, others down, etc. It happens. That is why we have style sheets so that these things can be adjusted. The PR is up and let's see what Kelvin says and take it from there,

from siemens_to_ismrmrd.

neurolabusc avatar neurolabusc commented on August 14, 2024

@hansenms it would be great to have sample images that demonstrate the need for various style sheets. If we can isolate the situations, we can usually get support from the manufacturer engineers to develop a formula without a style sheet. This sounds a lot like the Round_factor that GE uses which was completely mysterious until the GE engineers described the formula they use. Even if style sheets are required, it would be nice to demonstrate these edge cases for the community, helping other tools like dicm2nii and dcm2niix.

from siemens_to_ismrmrd.

hansenms avatar hansenms commented on August 14, 2024

Yeah, images are not really what we are looking for here, since we care about the interpretation of the raw data, which may yield different images depending on the reconstruction.

In terms of getting to a formula, then I don’t think it’s possible or even desirable in general. There are some classes of sequences that tend to yield similar interpretation of the parameters so we can probably cover most sequences with a few style sheets but there will be variations. Moreover, custom research sequences may have a completely different interpretation which may be a total hack but having the stylesheets allows users to accommodate that and still get usable reconstructions. I don’t see any specific advantage in trying compile in a formulate. The stylesheets are a good trade off between between being prescriptive and flexibility.

That said, there are repetitive sections in stylesheets and some kind of include function would be nice so we don’t have to edit the same thing in many places.

And example datasets are of course great to illustrate different cases, so we could do more there. But critically this should not be images but raw datasets.

On GE they are in much much worse shape when it comes to getting close to a formula. And most GE raw datasets cannot be interpreted without actually “knowing” what the sequence did. There is nothing in the dataset to even apply a stylesheets to. That’s why the GE converter also requires a sequence specific compiled component to interpret the profile ordering. Rounding is the least of our worries there ;)

from siemens_to_ismrmrd.

kspaceKelvin avatar kspaceKelvin commented on August 14, 2024

Heh, my bad for not updating all of the stylesheets when I changed it in #18 . This interpretation is correct for all sequences as of the current software version.

from siemens_to_ismrmrd.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.