[BUG] The website is broken

[etpeterson]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

The website that should be automatically generated is not working. https://osipi.github.io/TF2.4_IVIM-MRI_CodeCollection/

Screenshots [optional]

No response

Steps To Reproduce

Visit https://osipi.github.io/TF2.4_IVIM-MRI_CodeCollection/
Observe an error

Expected behavior

The website should be functional and should be updated when the pipeline runs.

Additional context

No response

Are you working on this?

None

[Unique-Usman]

@etpeterson, I checked the reason for it not working, the website deploy Github Action depends on workflow_run.

https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/.github/workflows/website.yml#L6

on:
  workflow_run:
    workflows: [Algorithm Analysis]

The workflow_run is currently failing due to the deprecated version of actions/upload-artifact: v3`.

Error: This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v3.

Will open a pr to update the version.

[etpeterson]

Thanks! Looks good.

[Unique-Usman]

@etpeterson, This is still not fixed yet due to the workflow_run. But, this time, it is because the artifact being existed before.

"
Run actions/upload-artifact@v4
Multiple search paths detected. Calculating the least common ancestor of all paths
The least common ancestor is /home/runner/work/TF2.4_IVIM-MRI_CodeCollection/TF2.4_IVIM-MRI_CodeCollection. This will be the root directory of the artifact
With the provided path, there will be 2 files uploaded
Artifact name is valid!
Root directory input is valid!
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run
"

Maybe you could deleted the artifact or we could change the retention time ?

[etpeterson]

@etpeterson, This is still not fixed yet due to the workflow_run. But, this time, it is because the artifact being existed before.

"
Run actions/upload-artifact@v4
Multiple search paths detected. Calculating the least common ancestor of all paths
The least common ancestor is /home/runner/work/TF2.4_IVIM-MRI_CodeCollection/TF2.4_IVIM-MRI_CodeCollection. This will be the root directory of the artifact
With the provided path, there will be 2 files uploaded
Artifact name is valid!
Root directory input is valid!
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run
"

Maybe you could deleted the artifact or we could change the retention time ?

I think we can just set overwrite to true. We want to update those files anyway.
https://github.com/actions/upload-artifact

[Unique-Usman]

k we can just set overwrite to true. We wa

Yeah, that is true. Let me send a pr for that.

[AhmedBasem20]

Hi, I was exploring this issue a few days ago and made some attempts that might help @Unique-Usman with his PR.
I think adding the overwrite option, will lead to losing all the data and keeping only the last file on the matrix.
My attempt was to create a unique artifact for each job on the matrix, and in the merge step, we extract the files from all the artifacts before merging them. This works with the new immutability constraints on upload-artifact

Code:

# build steps...

      - name: Upload raw data
        uses: actions/upload-artifact@v4
        with:
            name: Working_Data_${{ matrix.algorithm }}_${{ matrix.SNR }}
            retention-days: 1
            path: |
              test_output_${{ matrix.algorithm }}_${{ matrix.SNR }}.csv
              test_duration_${{ matrix.algorithm }}_${{ matrix.SNR }}.csv
            overwrite: true

  merge:
    runs-on: ubuntu-latest
    needs: build
    steps:
      - name: Download artifacts
        uses: actions/download-artifact@v4
        with:
          path: artifacts
          pattern: Working_Data_*
          merge-multiple: true
      - name: List downloaded files
        run: find artifacts -type f | sort
      - name: Merge fitting results
        run: |
          # Find the first output file to get the header
          FIRST_OUTPUT=$(find artifacts -name "test_output_*.csv" | head -n 1)
          head -n 1 "$FIRST_OUTPUT" > test_output.csv
          find artifacts -name "test_output_*.csv" -exec tail -q -n +2 {} \; >> test_output.csv

[AhmedBasem20]

You can find the run details for this solution here
The artifact issue is solved but it still fails because of another error on the compare step that is worth checking as well.

[etpeterson]

@Unique-Usman Seems like it's still failing and @AhmedBasem20 might have an idea?

[Unique-Usman]

Thanks @AhmedBasem20 and @etpeterson , I will take a look at it.

[Unique-Usman]

@AhmedBasem20 and @etpeterson, I looked into, it seems to me it is as a result of some non-present test reference file or incorrect content. I might need you guys to take a look also. Thank.

[etpeterson]

Not sure I understand the question/ask. Where are you seeing/testing it?

[Unique-Usman]

@etpeterson I means you should take a look at the action run by @AhmedBasem20 on his fork https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/13660567625

[AhmedBasem20]

@etpeterson To add more context, there are two issues in the analysis pipeline:

1. the upload artifact action on build job, which is solved on this run on my fork
2. the error in the compare job which is @Unique-Usman referring to, seems to be related to the changes made on Wrapper dev #77 because this issue first appeared right after merging this PR (run URL).

If possible, the analysis workflow should also run on pull request event to ensure it passes before merging, correct?

[etpeterson]

@AhmedBasem20 thanks for adding context.

Regarding 1. I took a look and it seems like a good fix. I think only the changes to the uploader and downloader are needed. The merging changes aren't necessary, correct? They both seem to be doing the same thing.

Regarding 2. I totally missed that it started to fail so long ago! Not great on my part. I think that's a real failure of the compare script. It could be the script needs to be updated, but it looks like one or more of the algorithms are not performing well anymore. I attached the results and you can see where it's FALSE it failed the comparison. Could try following this https://github.com/OSIPI/TF2.4_IVIM-MRI_CodeCollection/blob/main/tests/IVIMmodels/unit_tests/compare.r#L6 but I think it might be more than just a simple failure and might need some debugging.

test_results.csv

[etpeterson]

Oh also, the analysis pipeline is too long to run for every PR. It would be nice to but it just takes too long. Maybe there's a way to speed it up or do a fast version?

[Unique-Usman]

Oh also, the analysis pipeline is too long to run for every PR. It would be nice to but it just takes too long. Maybe there's a way to speed it up or do a fast version?

While there could be speed up, I do not think it is too long for PR. I have seen some projects PRs taking longer. Having it will really safe us lot of headache in the that arises as a result of not having it.