search

bids-apps / freesurfer

BIDS app wrapping recon-all from FreeSurfer
Apache License 2.0
40 stars 35 forks source link

Seems that some tests for FS7 are failing because they hit the wall time? #78

Closed Remi-Gau closed 1 year ago

Remi-Gau commented 1 year ago

https://app.circleci.com/pipelines/github/bids-apps/freesurfer/111/workflows/00c61725-eefe-4eb6-a748-c4b3586ce2fe/jobs/445

Shotgunosine commented 1 year ago

Persisting the image is taking too long it looks like. Might need to see if we can streamline the docker file.

PeerHerholz commented 1 year ago

Looks like the image builds but the workspace for the tests runs out. CI suggests to use parallelism for tests. I have never done that, anyone has an idea how to implement this?

Remi-Gau commented 1 year ago

other option: we do not persist anything to the workspace and just only ever get stuff from the cache in the follow up steps?

Shotgunosine commented 1 year ago

Sorry, it's building the image that's taking 42 minutes. I'll try a large or x-large machine to see if that speeds things up. If that doesn't work. We could download freesurfer in a previous step and persist it, then copy it into the container instead of downloading it. That might speed up the bulid, but it'd require some editing of the dockerfiles.

Remi-Gau commented 1 year ago

we may want to check if switching to github action should be considered.

Shotgunosine commented 1 year ago

It's not clear to me why the Freesurfer 7 build is now taking half an hour. It was taking ~16 minutes on previous commits and I don't see any changes in the dockerfile in the mean time. @Remi-Gau switching over to github actions would be one option, but I don't have time to do an overhaul like that right now.

@PeerHerholz, I don't think the length of time it's taking to build is something that parallelism is likely to solve.

It looks like the large machine might just barely sneak through. If that fails I'll try an x-large machine and see if it goes any faster, but I think we might need to cache the freesurfer tar if we want to get a more reliable build time.

Remi-Gau commented 1 year ago

Something is indeed strange.

2 circle CI runs on the same commit:

Shotgunosine commented 1 year ago

Ah, so there are intermittent timeout failures in both the build step and in the test step. Those test failures could maybe be fixed with parallel, or by splitting the tests into two steps.

On Thu, Sep 7, 2023 at 2:02 PM Remi Gau @.***> wrote:

Something is indeed strange.

2 circle CI runs on the same commit https://github.com/bids-apps/freesurfer/commit/466355952a4a32fd1a11f7db6d56ebbfe302baf5 :

— Reply to this email directly, view it on GitHub https://github.com/bids-apps/freesurfer/issues/78#issuecomment-1710570602, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABILLGFO4RCFIMDAN3V7S4TXZID4TANCNFSM6AAAAAA4H5CW3A . You are receiving this because you commented.Message ID: @.***>

Shotgunosine commented 1 year ago

I've fixed the build timeouts for now by skipping the cacheing step in #77.

The test timeouts are going to be a bit trickier and should be saved for a separate PR.

Remi-Gau commented 1 year ago

@Shotgunosine If you are not going to work on it right now, I may PR something to make a push to main triggers a push of "unstable" tag version to docker hub.

See an example of another app here: https://hub.docker.com/r/bids/antscorticalthickness/tags

This allows users to try the "bleeding" edge of the app.

Shotgunosine commented 1 year ago

Does this not already do that? https://github.com/bids-apps/freesurfer/blob/62eb27608575ff403a78f224b71102d73fadcc7f/.circleci/config.yml#L205-L207

Remi-Gau commented 1 year ago

ah yes sorry I missed that

But it seems that there is no deploy planned on this push to master

https://app.circleci.com/pipelines/github/bids-apps/freesurfer/118/workflows/3bda51f6-b0c5-4a55-b054-0fa773c0d33e

Remi-Gau commented 1 year ago

I suspect it is because we are ignoring deploys on all branches:

https://github.com/bids-apps/freesurfer/blob/62eb27608575ff403a78f224b71102d73fadcc7f/.circleci/config.yml#L274

and we only do it on tags

Shotgunosine commented 1 year ago

Gotcha, I’m not planning on working on this anymore today, feel free.

On Thu, Sep 7, 2023 at 4:06 PM Remi Gau @.***> wrote:

I suspect it is because we are ignoring deploys on all branches:

https://github.com/bids-apps/freesurfer/blob/62eb27608575ff403a78f224b71102d73fadcc7f/.circleci/config.yml#L274

and we only do it on tags

— Reply to this email directly, view it on GitHub https://github.com/bids-apps/freesurfer/issues/78#issuecomment-1710707277, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABILLGA7IESC62SKH2M6ZJ3XZISOHANCNFSM6AAAAAA4H5CW3A . You are receiving this because you were mentioned.Message ID: @.***>