Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CI/CD Pipeline setup #19

Closed
wyli opened this issue Jan 10, 2020 · 3 comments · Fixed by #36
Closed

CI/CD Pipeline setup #19

wyli opened this issue Jan 10, 2020 · 3 comments · Fixed by #36
Assignees
@atbenmurray @yanchengnv

Comments

@wyli
Copy link
Contributor

wyli commented Jan 10, 2020

No description provided.

@wyli wyli mentioned this issue Jan 13, 2020
Closed
@atbenmurray
Copy link
Contributor

atbenmurray commented Jan 14, 2020 •
edited
Loading

Yan, I'm taking the liberty of copying your initial email here as it is the best place for discussion

Hi Ben,

This item was assigned to you and me. If you don’t mind, I can do some research on possible solutions and put together a proposal.

But we need to first define some requirements. Here are what I can think of:

When PR merge request is received:
Check and make sure the PR has been reviewed and approved.
Run code style check and email result to author
Run all unit tests and make sure nothing is broken
Run integration tests and make sure nothing is broken
Notify team if the PR is accepted
Nightly build?

Please feel free to add/change.

Depending on the solution options we have, we may need to adjust these.
Let me know what you think.
Thanks,
Yan

@atbenmurray
Copy link
Contributor

We have been using 'GitLab CI/CD for GitHub' as it comes with full GPU test agent support. Shall we just go with that?

Let's protect master so that it can only be updated via PR.

Github provides 'auto-squash' option on PRs so that they are always a single commit on master. This is good because it keeps master history clean. I'd go further and suggest rewriting commit message for the PR so that it summarises the feature.

Re checks, we should be able to configure CI so that testing happens automatically on PR submission (i.e. before it has been passed/merged). All unit and integration tests should be run on a CI basis, ideally.

We can have a nightly build. We may wish to have longer running training tests that execute on it. It is a bit tricky given the unstable nature of some networks, however.

Notification of the team; I guess you mean automatic emails; don't we get them from the issues, updates to the issues? If not, I guess that there is a hook of some kind.

@wyli
Copy link
Contributor Author

wyli commented Jan 15, 2020

Thanks @atbenmurray, just to confirm that now the master can only be updated via PR, squash merging is enabled for PR.

@IsaacYangSLA IsaacYangSLA mentioned this issue Jan 17, 2020
Merged
@wyli wyli mentioned this issue Jan 17, 2020
Closed
wyli pushed a commit that referenced this issue Jan 17, 2020
@IsaacYangSLA @wyli
Create .gitlab-ci.yml (#30)
eef2941 
an initial step towards #19
Nic-Ma pushed a commit that referenced this issue Jan 21, 2020
@wyli @ericspod
@IsaacYangSLA @Nic-Ma
9 part a adding test intensity normalisation transform (#33)
e31ec10 
* Adding script to run unit tests and example test cases (#29)

Adding script to run unit tests and example test cases

* initial unit tests for dice loss (#27)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* initial unit tests for 2d/3d unet (#26)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* 14 code examples of monai input data pipeline (#24)

* fixes cardiac example

* update example cardiac segmentation

* Create .gitlab-ci.yml (#30)

an initial step towards #19

* tests intensity normalizer

- revised to support both `[key]` and `key` as an input for apply_keys
- added `NumpyImageTestCase2D` and `TorchImageTestCase2D`

* style updates and new test cases:

- adding copyright notice
- validate user input before setting class member
- one line space after copyright
- testing multiple keys input data

Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: Isaac Yang <isaacy@nvidia.com>
ericspod added a commit that referenced this issue Jan 22, 2020
@Nic-Ma @wyli
@ericspod @IsaacYangSLA
9 intensity normalisation transform (#25)
1390701 
* [DLMED] implement intensity normalization transform

design according to our latest discussion:
1. input data is dict format with keys for fields.
2. only based on PyTorch and data shape is channel_last.

* 9 part a adding test intensity normalisation transform (#33)

* Adding script to run unit tests and example test cases (#29)

Adding script to run unit tests and example test cases

* initial unit tests for dice loss (#27)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* initial unit tests for 2d/3d unet (#26)

* initial unit tests for 2d/3d unet
* unit tests update
 - triggering unit tests via github workflow
 - renamed testconvolutions.py to test_convolutions.py
 - test unet test cases as variables for readability

* 14 code examples of monai input data pipeline (#24)

* fixes cardiac example

* update example cardiac segmentation

* Create .gitlab-ci.yml (#30)

an initial step towards #19

* tests intensity normalizer

- revised to support both `[key]` and `key` as an input for apply_keys
- added `NumpyImageTestCase2D` and `TorchImageTestCase2D`

* style updates and new test cases:

- adding copyright notice
- validate user input before setting class member
- one line space after copyright
- testing multiple keys input data

Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: Isaac Yang <isaacy@nvidia.com>

* [DLMED] simplify intensity normalization transform for MVP

Co-authored-by: Wenqi Li <wenqi.li@ucl.ac.uk>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
Co-authored-by: Isaac Yang <isaacy@nvidia.com>
@wyli wyli mentioned this issue Jan 24, 2020
Merged
@wyli wyli closed this as completed in #36 Feb 4, 2020
@wyli wyli mentioned this issue Feb 6, 2020
Merged
wyli pushed a commit that referenced this issue Apr 9, 2021
@Nic-Ma
Merge pull request #19 from Project-MONAI/master
42a45e0 
merge master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@atbenmurray atbenmurray

@yanchengnv yanchengnv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

11 integration test resolves conflicts Project-MONAI/MONAI
3 participants
@wyli @atbenmurray @yanchengnv