Generate a variety of testing data and came up command_line version of sim_vim_sig.py

[Unique-Usman]

A clear and concise description of what you want to happen Added this two files. sim_ivim_sig_command_line.py - This is the command line version of the sim_vim_sig,py sim_ivim_sig_mul_bval.py - This version is used for running multiple b_value which are stored in b_values.py

Various test files based on the multiple value of b_value in the b_values.py ├── generic_five.json ├── generic_four.json ├── generic_original.json ├── generic_three.json ├── generic_two.json

Also, the generic_original.json was supposed to be the same thing as generic.json. But, the result is not the same. I guess the generic.json was generate from another b_value different from the current one.

Also, @etpeterson what do you think about combining the three scripts(sim_ivim_sig_command_line.py, sim_ivim_sig_mul_bval.py, sim_ivim_sig.py) together instead of having multiple scirpts doing almost similar thing ? What we can do is to just have a single script that can be used as command line tools and is able to generate multiple value of test data based on the multiple b_values.

I will update the pr based on your feeedback and also come up with a documentation for running the scipt.

Link this PR to an issue [optional]

Fixes #41

Checklist

• [ ] Self-review of changed code
• [ ] Added automated tests where applicable
• [ ] Update Docs & Guides

[etpeterson]

Yes, I think it should just be one script, just changes to the original sim_ivim_sig.py would be great. One of the inputs could be a choice for bvalues to generate.

The change to generic.json looks like somehow the newline at the end got dropped, but the contents are the same. It's hard to say how this happened because it's difficult to see changes with the duplicated script. Once everything is all together then it'll be easier to look at the changes made.

[Unique-Usman]

A clear and concise description of what you want to happen I have converted the script to one sim_ivim_sig.py. Now it is able to take multiple bvalues through a json file. Also it can also take a single value through command line. The default value is also present.

Various test files based on the multiple value of b_value in the b_values.py ├── generic_0.json ├── generic_1.json ├── generic_2.json ├── generic_3.json ├── generic_4.json

Also, the generic_0.json was supposed to be the same thing as generic.json as I used the placeholder bvalue for it. But, the result is not the same. I guess the generic.json was generate from another b_value different from the current one. (I did not make any change to generic.json.)

There might also be a need for me to explain the changes made through a call or recording @etpeterson

I will update the pr based on your feeedback and also come up with a documentation for running the scipt.

Link this PR to an issue [optional] Fixes https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/issues/41

Checklist Self-review of changed code Added automated tests where applicable Update Docs & Guides

[Unique-Usman]

@etpeterson Kindly go through the comment, then I can make the necessary changes. Thank you.

[Unique-Usman]

I changed the generic_0.json. Also correct the mistake in the script. Also changed the -bf option flag to -f flag.

What I was saying regarding the generic.json is that, the value in generic_0.json was supposed to be the same thing with the generic.json. Because, the bvalue for generic_0.json was the one bvalue in the repo. My suggestion is that, the bvalue used to generate generic.json is different from the one present in the repo.

[etpeterson]

What I was saying regarding the generic.json is that, the value in generic_0.json was supposed to be the same thing with the generic.json. Because, the bvalue for generic_0.json was the one bvalue in the repo. My suggestion is that, the bvalue used to generate generic.json is different from the one present in the repo.

Is this still different? We can see the bvalues used for generic.json in the file itself. And I can see they are the same as the default set you're using. Is it different on the main branch too? I would be surprised if it's different on the main branch so that would be a good place to check.

[Unique-Usman]

@etpeterson , I ran it on the unchaged main branch and it still give the different result from the generic.json. Yeah, the bvalues used is the same, something is wrong somewhere I guess.

[etpeterson]

@etpeterson , I ran it on the unchaged main branch and it still give the different result from the generic.json. Yeah, the bvalues used is the same, something is wrong somewhere I guess.

That is surprising. What's the difference?

[Unique-Usman]

All the decimal values

[etpeterson]

This is strange. I don't know why that would change. The history doesn't show meaningful changes to that file. I'll give it a shot.

Also, unfortunately you have a merge conflict.

[etpeterson]

I'm not sure what to say, but it worked for me, I got the same data in both. Here are my steps.

• from the phantoms/MR_XCAT_qMRI folder
• git checkout main
• git pull
• python sim_ivim_sig.py
• let it run and in a minute or two it finished
• git status shows no changes
• diff generic.json ../../tests/IVIMmodels/unit_tests/generic.json shows no changes

[Unique-Usman]

Did you move the utitlities to the phantoms/MR_XCAT_qMRI when running the code ?

[etpeterson]

Did you move the utitlities to the phantoms/MR_XCAT_qMRI when running the code ?

No, if the pythonpath is set correctly it finds them where they are. Even if you pull the images manually I think the result should be the same though.

[Unique-Usman]

Yeah, I was able to get the same data. But, do we want to add https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/phantoms/MR_XCAT_qMRI/sim_ivim_sig.py#L13 to the script. This keeps downloading all the data everytime.

[etpeterson]

This keeps downloading all the data everytime.

If that's the case then it sounds like a good issue to be fixed in a different pull request.

[etpeterson]

I'll try to actually run it later and test that, but the code looks good to me at the moment. Just a few changes still pending.

[etpeterson]

Yeah, I was able to get the same data

What was the problem? Is it something that we can fix with some instructions or code?

[Unique-Usman]

This keeps downloading all the data everytime.

If that's the case then it sounds like a good issue to be fixed in a different pull request.

I think, if possible, we should have the downloading of data separately. Then, in the documentation for running the code that I will be providing, I will mention it there.

[Unique-Usman]

f that's the case then it sounds like a good issue to be fixed in a different pull request.

I really think it is some mix up somewhere, probably I deleted or add some code mistakenly. I resolved the problem by rewriting the code from the start.

[etpeterson]

I think, if possible, we should have the downloading of data separately. Then, in the documentation for running the code that I will be providing, I will mention it there.

We shouldn't try to reimplement git-lfs here, but also a simple check to see if the requisite files are present and if not then download seems pretty easy. If it's in the document and in the code then requiring two steps is fine.