Added a yml file to build the docker image #74

Unique-Usman · 2024-08-11T20:52:04Z

Create a github action for building and running the docker image.

@etpeterson, hello, how are you doing and how have you been ?

Should we make the github action to build the image and also run it or just building. For running, we will need some data, should we download the data during in the github action or add the data as part of the repository ?

etpeterson

I really like this direction. Just a few comments but I think it's really close already.

.github/workflows/docker-build-and-run.yml

etpeterson · 2024-08-12T14:16:55Z

.github/workflows/docker-build-and-run.yml

+          docker run -it --rm --name TF2.4_IVIM-MRI_CodeCollection \
+            -v ${{ github.workspace }}:/usr/src/app \
+            -v ${{ github.workspace }}:/usr/app/output \
+            tf2.4_ivim-mri_codecollection brain.nii.gz brain.bvec brain.bval


I don't think the data are available. We could pull it, but that run would take a while. Maybe we could read a small text file that does just a few fits and takes just a second or two. We really just want to run it to confirm it works, we have other tests to check the actual results.

How to go about the reading the small text file ?

Perhaps even easier to simulate something. You can generate signals using from utilities.data_simulation.GenerateData import GenerateData. It's done in test_ivim_synthetic.py for example. Then save it to a nifti using the function you wrote, then read the nifti. Maybe just a 10x10x1 or something small and fast. B-values of [0, 50, 100, 200, 500, 1000] with maybe one or two vectors.

.github/workflows/docker-build-and-run.yml

etpeterson · 2024-08-12T14:19:37Z

Create a github action for building and running the docker image.

@etpeterson, hello, how are you doing and how have you been ?

Should we make the github action to build the image and also run it or just building. For running, we will need some data, should we download the data during in the github action or add the data as part of the repository ?

Doing well and thanks for coming back to this. I submitted a review for you.

etpeterson · 2024-08-20T03:59:22Z

.github/workflows/docker-build-and-run.yml

+          docker run -it --rm --name TF2.4_IVIM-MRI_CodeCollection \
+            -v ${{ github.workspace }}:/usr/src/app \
+            -v ${{ github.workspace }}:/usr/app/output \
+            tf2.4_ivim-mri_codecollection brain.nii.gz brain.bvec brain.bval


Perhaps even easier to simulate something. You can generate signals using from utilities.data_simulation.GenerateData import GenerateData. It's done in test_ivim_synthetic.py for example. Then save it to a nifti using the function you wrote, then read the nifti. Maybe just a 10x10x1 or something small and fast. B-values of [0, 50, 100, 200, 500, 1000] with maybe one or two vectors.

etpeterson · 2024-08-20T04:01:54Z

.github/workflows/docker-build-and-run.yml

+
+      - name: Save Docker image to a tarball
+        run: |
+          docker save -o tf2.4_ivim-mri_codecollection.tar tf2.4_ivim-mri_codecollection


These things are large, how about also zipping it.

Unique-Usman · 2024-08-24T09:42:32Z

@etpeterson

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L25

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/tests/IVIMmodels/unit_tests/test_ivim_synthetic.py#L23C4-L23C34

How do we get the ivim_data ?

etpeterson · 2024-08-26T04:04:41Z

@etpeterson

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L25

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/tests/IVIMmodels/unit_tests/test_ivim_synthetic.py#L23C4-L23C34

How do we get the ivim_data ?

I'm just saying random but realistic f [0, 1], D [0, 1e-3], D* [0, 1e-1], generate b-value curves from that using the GenerateData that you linked to and the b-values I suggested above. Generate that volume and use that as a test dataset.

Unique-Usman · 2024-08-28T13:17:15Z

@etpeterson, I think I might need your regarding this part. Thank you. I am able to get it.

etpeterson · 2024-08-29T00:50:47Z

@etpeterson, I think I might need your regarding this part. Thank you. I am able to get it.

Something roughly like this. I haven't tested it but it has the basics.

gd = GenerateData()
f_in = np.random.uniform(low=0, high=1, size=(10, 10, 5))
D_in = np.random.uniform(low=0, high=1e-3, size=(10, 10, 5))
Dp_in = np.random.uniform(low=0, high=1e-1, size=(10, 10, 5))
bvals = [0, 50, 100, 500, 1000]
images = gd.ivim_signal(self, D_in, Dp_in, f_in, S0, bvalues)
save_nii(images)

Unique-Usman · 2024-08-29T21:03:29Z

@etpeterson, I think I might need your regarding this part. Thank you. I am able to get it.

Something roughly like this. I haven't tested it but it has the basics.
gd = GenerateData()
f_in = np.random.uniform(low=0, high=1, size=(10, 10, 5))
D_in = np.random.uniform(low=0, high=1e-3, size=(10, 10, 5))
Dp_in = np.random.uniform(low=0, high=1e-1, size=(10, 10, 5))
bvals = [0, 50, 100, 500, 1000]
images = gd.ivim_signal(self, D_in, Dp_in, f_in, S0, bvalues)
save_nii(images)

Did something similay earlier. Apparently the error seems to come from GenerateData due to shape imcompability.

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L56

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L141

etpeterson · 2024-08-30T04:54:57Z

@etpeterson, I think I might need your regarding this part. Thank you. I am able to get it.

Something roughly like this. I haven't tested it but it has the basics.
gd = GenerateData()
f_in = np.random.uniform(low=0, high=1, size=(10, 10, 5))
D_in = np.random.uniform(low=0, high=1e-3, size=(10, 10, 5))
Dp_in = np.random.uniform(low=0, high=1e-1, size=(10, 10, 5))
bvals = [0, 50, 100, 500, 1000]
images = gd.ivim_signal(self, D_in, Dp_in, f_in, S0, bvalues)
save_nii(images)
Did something similay earlier. Apparently the error seems to come from GenerateData due to shape imcompability.

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L56

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/utilities/data_simulation/GenerateData.py#L141

What's the error specifically? I'm not sure if I've ever tried GenerateData with multiple values at a time, but it doesn't seem like the lines you linked should be causing problems.

Unique-Usman · 2024-08-30T06:14:28Z

Unique-Usman · 2024-08-30T06:15:11Z

assert D >= 0, 'D must be >= 0'

etpeterson · 2024-08-30T12:38:03Z

assert D >= 0, 'D must be >= 0'

Ok, so this assert is correct except that it should be in numpy format, so assert np.all(D>=0).

Unique-Usman · 2024-09-01T23:14:25Z

So, I made this changes, but, it seems there are multiple bugs in the GenerateData.py.

etpeterson · 2024-09-02T01:54:13Z

So, I made this changes, but, it seems there are multiple bugs in the GenerateData.py.

Could be, but hard to tell just from that. Your inputs are all the same size?

etpeterson · 2024-09-24T03:45:18Z

Hey, just checking - are you still working on this?

Unique-Usman · 2024-09-24T08:30:21Z

@etpeterson, I have resumed working on it, should get back in few minutes.

… for generating the sample data. Zipped the docker image generated. Make changes to nifit_wrapper to use 4, 4

Unique-Usman · 2024-09-26T00:54:41Z

@etpeterson kindly go through this whenever you are chanced.

etpeterson

Looking good. A few more comments.

etpeterson · 2024-09-26T04:33:35Z

utilities/data_simulation/np

What is this file?

Some erroneus files, will remove it.

etpeterson · 2024-09-26T04:39:42Z

Docker/generate_signal_docker_test.py

+import nibabel as nib
+from utilities.data_simulation.GenerateData import GenerateData
+
+def save_nii(data, filename='ivim_image.nii.gz'):


There's already a save_nifti_file in the wrapper.

Yeah, sure.

etpeterson · 2024-09-26T04:43:53Z

Docker/generate_signal_docker_test.py

What is the purpose of this file? It looks like it generates an image but I don't see it being used for anything. I was expecting to see the simulated image and associated files generated by this.

This is the code I used for generating the signal which I was using for the docker. I did not make use of it because I already generated the images. I will restructured it to generate the images on the fly thereby making use of it in the build process.

etpeterson · 2024-09-26T04:44:52Z

Docker/ivim_simulation.nii.gz

This and the associated bval and bvec files are inside the docker folder, so they're getting added to the docker image. We don't want that. Also, shouldn't they be generated on the fly rather than be pre-generated?

I will generate it on the fly, I thought it makes sense having it constant.

etpeterson · 2024-09-26T04:46:30Z

.github/workflows/docker-build-and-run.yml

+
+      - name: Run Docker container
+        run: |
+          docker run --rm --name TF2.4_IVIM-MRI_CodeCollection \


We should confirm that images are actually generated. Maybe not the exact values in the images, but even that the files we expect exist.

Yeah, exactly.

…ation to ensure files are being generated

…nning the image generation script

Unique-Usman · 2024-09-27T04:23:23Z

@etpeterson, kindly take a look.

etpeterson

Looking good, really just those few changes and the question about the docker file artifact.

.github/workflows/docker-build-and-run.yml

etpeterson · 2024-10-01T04:10:21Z

.github/workflows/docker-build-and-run.yml

+        uses: actions/upload-artifact@v4
+        with:
+          name: docker-image
+          path: tf2.4_ivim-mri_codecollection.tar.gz


I'm not seeing why, but the artifact is empty. I wonder if it's too big? If there's no way to save it perhaps it could be split or maybe we just forget about saving the file for now.

I will check this and verify.

@etpeterson, I think it is there, where are you checking

A whole docker image is not 198 bytes. If you download it and unzip it you'll find it empty. Also, no space left on device:

@etpeterson, any suggested solution ?

You could try a cleaning
- name: Cleanup Docker run: | docker system prune -a --force

You could also try alpine linux base. I think it's smaller but might not work correctly.

If neither of those works then let's build and test but not push.

…e the uploading of the docker to artifact below docker run verification

Unique-Usman · 2024-10-21T07:33:56Z

@etpeterson, both approach did not work. When I did the cleanup, I was still getting empty files. Also, the alpine base does not support the use of torch which is part of the requirement. So, I am removing the push component

etpeterson · 2024-10-22T04:41:42Z

@etpeterson, both approach did not work. When I did the cleanup, I was still getting empty files. Also, the alpine base does not support the use of torch which is part of the requirement. So, I am removing the push component

Ok, seems like that's not possible then. You're still trying to upload the docker image though.

Also, please don't rewrite the history like you did with force pushes. It make it really hard to review because I can't see what you had before and what my previous comments relate to.

Unique-Usman · 2024-10-22T06:30:38Z

@etpeterson, both approach did not work. When I did the cleanup, I was still getting empty files. Also, the alpine base does not support the use of torch which is part of the requirement. So, I am removing the push component

Ok, seems like that's not possible then. You're still trying to upload the docker image though.

My bad, I thought I removed this. I should have verfied in the actions.

Also, please don't rewrite the history like you did with force pushes. It make it really hard to review because I can't see what you had before and what my previous comments relate to.

I am sorry for this, I was avoiding this also.

Added a yml file to build the docker image

af49849

etpeterson requested changes Aug 12, 2024

View reviewed changes

Removed -it, and also save the docker image as an artifcat

c2149da

Unique-Usman requested a review from etpeterson August 13, 2024 19:48

etpeterson requested changes Aug 20, 2024

View reviewed changes

Unique-Usman requested a review from etpeterson August 24, 2024 09:42

Unique-Usman added 2 commits September 26, 2024 00:33

Added sample data for running the docker image, also added the script…

863e7f7

… for generating the sample data. Zipped the docker image generated. Make changes to nifit_wrapper to use 4, 4

Fixed the float not subscriptable error

1fa952c

etpeterson requested changes Sep 26, 2024

View reviewed changes

Unique-Usman added 3 commits September 26, 2024 07:38

Generated the testing images during docker building and added confirm…

7294bde

…ation to ensure files are being generated

Removed erroneus file

80c1663

Improve the github actions to install all the required library for ru…

602f17d

…nning the image generation script

Unique-Usman force-pushed the main branch from 4930e61 to 602f17d Compare September 26, 2024 08:07

Added nibabel to the requirement

33acf99

Unique-Usman force-pushed the main branch from b97d896 to 33acf99 Compare September 26, 2024 10:21

Unique-Usman force-pushed the main branch from 625c9a1 to dc9265d Compare September 26, 2024 11:15

Added debugging output options to the github action

7b2f0b8

Unique-Usman force-pushed the main branch 2 times, most recently from 056deca to 7b2f0b8 Compare September 26, 2024 11:38

Change the name of the ivim_image.nii.gz to ivim_simulation.nii.gz

ee2eb99

Unique-Usman force-pushed the main branch from a300014 to ee2eb99 Compare September 26, 2024 12:06

Unique-Usman requested a review from etpeterson September 26, 2024 12:16

etpeterson requested changes Oct 1, 2024

View reviewed changes

used docker buildx for save the docker image as tar file and also mov…

a4136f8

…e the uploading of the docker to artifact below docker run verification

Unique-Usman force-pushed the main branch from de6d34c to a4136f8 Compare October 1, 2024 05:45

Revert from archiving to saving then archiving

a6ddfb2

Unique-Usman force-pushed the main branch from 05026af to a6ddfb2 Compare October 1, 2024 06:03

Unique-Usman requested a review from etpeterson October 1, 2024 06:18

Added Docker cleanup

ea69e18

Unique-Usman force-pushed the main branch from f32f468 to ea69e18 Compare October 21, 2024 04:51

Change the base image to alpine linux

428095e

Unique-Usman force-pushed the main branch from 35313a7 to 428095e Compare October 21, 2024 07:13

Change the version of libssl to 3

6fee6ed

Unique-Usman force-pushed the main branch 2 times, most recently from c8e45e8 to c6a787d Compare October 21, 2024 07:31

Remove pushing of the image and used slim instead of alpine

1f6da96

Unique-Usman force-pushed the main branch from 0195905 to 1f6da96 Compare October 21, 2024 07:36

Merge branch 'main' into main

17b98f8

Avoid uploading the docker image

12ae83f

etpeterson approved these changes Oct 28, 2024

View reviewed changes

etpeterson merged commit 5c217fc into OSIPI:main Oct 28, 2024
13 checks passed

Added a yml file to build the docker image #74

Added a yml file to build the docker image #74

Conversation

Unique-Usman commented Aug 11, 2024

Create a github action for building and running the docker image.

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson commented Aug 12, 2024

Create a github action for building and running the docker image.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Unique-Usman commented Aug 24, 2024

etpeterson commented Aug 26, 2024

Unique-Usman commented Aug 28, 2024

etpeterson commented Aug 29, 2024

Unique-Usman commented Aug 29, 2024

etpeterson commented Aug 30, 2024

Unique-Usman commented Aug 30, 2024

Unique-Usman commented Aug 30, 2024

etpeterson commented Aug 30, 2024 • edited Loading

Unique-Usman commented Sep 1, 2024 • edited Loading

etpeterson commented Sep 2, 2024

etpeterson commented Sep 24, 2024

Unique-Usman commented Sep 24, 2024

Unique-Usman commented Sep 26, 2024

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Unique-Usman Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Unique-Usman commented Sep 27, 2024

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Unique-Usman commented Oct 21, 2024

etpeterson commented Oct 22, 2024

Unique-Usman commented Oct 22, 2024 • edited Loading

etpeterson commented Aug 30, 2024 •

edited

Loading

Unique-Usman commented Sep 1, 2024 •

edited

Loading

Unique-Usman Sep 26, 2024 •

edited

Loading

etpeterson Oct 2, 2024 •

edited

Loading

Unique-Usman commented Oct 22, 2024 •

edited

Loading