NeuroDesk / neurocontainers

The containers can be used in combination with our transparent singularity or neurocommand tool, that wrap the executables inside a container to make them easily available for pipelines
https://www.neurodesk.org
MIT License
19 stars 49 forks source link

FYI: --matlabmcr option is apparently not available in neurodocker anymore #57

Closed civier closed 2 years ago

civier commented 2 years ago

@DavidjWhite33 FYI

Building containers with matlab MCR does not work because neurodocker does not receive anymore the --matlabmcr option.

It is not listed in the help of neurodocker: https://www.repronim.org/neurodocker/user_guide/cli.html And is indeed not in the github code of neurodocker, as far as I can see: https://github.com/ReproNim/neurodocker

It did exist in the documentation of older versions: https://hub.docker.com/r/repronim/neurodocker/ https://hub.docker.com/r/kaczmarj/neurodocker

I'm not sure why the change, but I opened an issue in neurodocker: https://github.com/ReproNim/neurodocker/issues/421 Let's see what they say.

@stebo85 @aswinnarayanan Is our neurodocker fork always is in sync with neurodocker master, or do you sync it once in a while? What would be the better option?

stebo85 commented 2 years ago

They seem to install the matlab MCR manually in their SPM template: https://github.com/ReproNim/neurodocker/blob/master/neurodocker/templates/spm12.yaml

We manually sync the fork when needed.

civier commented 2 years ago

Yes. I found the same re spm12. Wonder why they made the change?

Is there any way to trigger a test rebuild (that doesn't actually update the packages repository, but only checks the exit codes) of all containers every time we sync neurodocker? This will ensure that the syncing does not break anything. Or is it too expensive? what are the limits on our CI resources?

On Wed, Sep 22, 2021 at 8:45 PM Steffen Bollmann @.***> wrote:

They seem to install the matlab MCR manually in their SPM template: https://github.com/ReproNim/neurodocker/blob/master/neurodocker/templates/spm12.yaml

We manually sync the fork when needed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeuroDesk/neurocontainers/issues/57#issuecomment-924812717, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7FUI2VTNOAGSXBNSJI2OTUDGXU7ANCNFSM5EQ2ZRRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

stebo85 commented 2 years ago

it's possible, you could add this check in the gitlab actions run. There is currently no limit on CI.

civier commented 2 years ago

Ok. I'll have to learn how to do that. I'll add it to my to do list.

-- Oren Civier Neuroimaging Informatics Fellow Swinburne Neuroimaging | National Imaging Facility https://anif.org.au/team/oren-civier/ https://www.swinburne.edu.au/neuroimaging

Swinburne University of Technology Hawthorn Campus, Advanced Technologies Centre, Room 914 +61 3 9214 4628 | +61 4 3101 6603 @.*** (Email/Teams video call) https://twitter.com/orencivier

On Wed, 22 Sep 2021, 21:41 Steffen Bollmann, @.***> wrote:

it's possible, you could add this check in the gitlab actions run. There is currently no limit on CI.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NeuroDesk/neurocontainers/issues/57#issuecomment-924849290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7FUI3222U6SA4DWC5Z65TUDG6GXANCNFSM5EQ2ZRRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

stebo85 commented 2 years ago

This has been added back to neurodocker: https://github.com/ReproNim/neurodocker/pull/422

civier commented 2 years ago

Hello @stebo85

We tested the new version of Neurodocker that they want to merge (it actually hasn't been merged yet). It does have Matlab MCR, but apparently, is missing other required utils. So @DavidjWhite33 , @TomEmotion and I are trying to go back to a proper Neurodocker version like the one you used when building CONN.

You built CONN last on 22/3/2021: https://github.com/NeuroDesk/neurocontainers/pkgs/container/caid%2Fconn_20b

This is the last commit of Neurocontainers on 22/3/2021: https://github.com/NeuroDesk/neurocontainers/tree/6215a0fcf540a6f69dbb9f4afa55228d92c77170

The main_setup.sh file on that commit was: https://github.com/NeuroDesk/neurocontainers/blob/6215a0fcf540a6f69dbb9f4afa55228d92c77170/recipes/main_setup.sh

When I go to our fork of Neurodocker https://github.com/NeuroDesk/neurodocker

... and press commits, I see the history of commits in the ReproNim/neurodocker mixed with commits in NeuroDesk/neurodocker https://github.com/NeuroDesk/neurodocker/commits/master

Is there any way to investigate the history of the fork, in order to find out what was the state of the fork (to what commit ID of Neurodocker/master it was synchronised), when you built CONN? Also -- do you ever do any commits directly to NeuroDesk/neurodocker? Are they getting deleted when you sync with ReproNim/neurodocker? Is there any trace of them?

Thanks again for your assistance. As you see, we still learn all the fine details of Github. Oren

civier commented 2 years ago

@stebo85 Notice that I edited the previous comment!

aswinnarayanan commented 2 years ago

Hi @civier. You can find the github commit that created almost any of the neurocontainers using docker inspect.

docker inspect ghcr.io/neurodesk/caid/conn_20b:20210322

The information is contained in the labels

            ],
            "OnBuild": null,
            "Labels": {
                "GITHUB_REPOSITORY": "NeuroDesk/caid",
                "GITHUB_SHA": "6215a0fcf540a6f69dbb9f4afa55228d92c77170"
            }
        },

So you've got the correct one

stebo85 commented 2 years ago

@civier and @aswinnarayanan - I had to reset the fork in neurodesk to the upstream one because we were not able to merge anymore. I kept a backup here: https://github.com/stebo85/neurodocker-old-fork - Oren, if you use the old fork conn should build again.

civier commented 2 years ago

@stebo85 Thanks for the information, and it's a pity the history of the fork is not readily available on the Github website. Do you know if there is any way to access it? If not, I think we better document this ourselves (luckily for this case, not long time has passed; but in the future, changes might be done by people who would later leave the project). (what @aswinnarayanan suggested above only gives the commit on neurodesk/caid, not neurodesk/neurodocker, right?)

TomEmotion commented 2 years ago

Thanks Steffen. Dave and I found your backup. Did you put in a pull request to repronim for the changes you made to get CONN working and it either wasn't merged or was subsequently deleted? In any case, Dave will grab the changes you made in there (mainly the relevant yaml files) and use them to get the MCR working again in Neurodocker. Once we have that working we'll submit a pull request to get it merged with the master branch.

aswinnarayanan commented 2 years ago

@civier. Yup I was just referring to the neurocontainers version

stebo85 commented 2 years ago

Yes, I had submitted multiple pull-requests to the neurodocker-upstream project, but they didn't get merged. Then the forks got out of sync and I had to reset it.

TomEmotion commented 2 years ago

Right. Frustrating! Is it worth us considering having our own NeuroDesk dev branch of important repos (such as Neurodocker) for any bugfixes or additions that we create, that we can use while waiting for pull requests to be merged in the origin repo? The alternative is for us each to have our own, but then it's hard for anyone else to see/find.

stebo85 commented 2 years ago

@TomEmotion - we have our own dev branch of Neurodocker - this is exactly what our fork is :) https://github.com/NeuroDesk/neurodocker/ - we just need to be careful to have all our additions in a "fork" branch so we can still pull down upstream updates while we wait for them to be merged

TomEmotion commented 2 years ago

Yes, exactly - that is what I was suggesting. In that way, for example, we would have a branch that includes the changes you made to get CONN built.

stebo85 commented 2 years ago

yes, I wrote Jakub and he will merge the latest pull requests in upstream. Then I will pull these changes into our fork. Then I will recreate all our changes in separate branches and I will also create a "fork" branch that we can use for our builds. If everything goes well this should be done by end of next week.

DavidjWhite33 commented 2 years ago

Thanks @stebo85 - from the looks of the merged changes in the Repronim repo, our only required changes will be to include more recent runtime versions (so we can disregard the matlabmcr.yaml I've added to our fork, apart from grabbing the links to more recent versions).

stebo85 commented 2 years ago

Yes, that looks good :)

I had to move your changes to a separate branch, otherwise we cannot sync with upstream. Do not commit changes to the master branch - always keep it in dedicated branches, otherwise we can not send pull requests or update from upstream :)

your branch is here now: https://github.com/NeuroDesk/neurodocker/tree/david-white-add-matlab-mcr

stebo85 commented 2 years ago

closing as MCR is added back in and we submitted the pull request