More CI checks #119
More CI checks #119
Conversation
Previously, tests were run with petric.py from main.py, meaning that we don't actually check any changes to it in a PR
| @@ -33,14 +33,22 @@ jobs: | |||
| shell: bash -el {0} | |||
| run: | | |||
| source /opt/SIRF-SuperBuild/INSTALL/bin/env_sirf.sh | |||
| curl -fsSL https://raw.githubusercontent.com/SyneRBI/PETRIC/main/petric.py > petric.py | |||
There was a problem hiding this comment.
we need this line to overwrite any users' changes as per the README
Any modifications to
petric.pyare ignored.
There was a problem hiding this comment.
sure, but does tha tuse this run.yml? I guess it does. But then this isn't really CI.
anyway.
There was a problem hiding this comment.
yes all submission repos are a fork of this one, so they all use this run.yml.
- pro: we all use the same
petric.py - con: it's impossible for organisers to test changes to
petric.pyin CI- I've had to manually test any change(s)
We could de-sync forks from this repo, but I'd rather not (it might confuse participants)
| @@ -33,14 +33,22 @@ jobs: | |||
| shell: bash -el {0} | |||
| run: | | |||
| source /opt/SIRF-SuperBuild/INSTALL/bin/env_sirf.sh | |||
| curl -fsSL https://raw.githubusercontent.com/SyneRBI/PETRIC/main/petric.py > petric.py | |||
| dataset=Siemens_mMR_NEMA_IQ | |||
| curl -sfSL -o ${dataset}.zip https://petric.tomography.stfc.ac.uk/data/${dataset}.zip | |||
There was a problem hiding this comment.
| curl -sfSL -o ${dataset}.zip https://petric.tomography.stfc.ac.uk/data/${dataset}.zip | |
| curl -fsSLO https://petric.tomography.stfc.ac.uk/data/${dataset}.zip |
| data = petric.get_data(srcdir) | ||
| assert data.OSEM_image.max() > 0 | ||
| assert data.reference_image.max() > 0 | ||
| assert data.FOV_mask.max() > 0 |
There was a problem hiding this comment.
may as well test init?
| assert data.FOV_mask.max() > 0 | |
| assert data.FOV_mask.max() > 0 | |
| algo = Submission(data) |
or maybe to avoid e.g. "could not find GPU"...
| assert data.FOV_mask.max() > 0 | |
| assert data.FOV_mask.max() > 0 | |
| try: | |
| algo = Submission(data) | |
| except Exception: | |
| from warnings import warn | |
| warn("Submission.__init__ failed", RuntimeWarning, stacklevel=2) |
There was a problem hiding this comment.
we'd want to have a dummy submission then. Not sure what a test tells us that we know will fail. Anyway, that'll all be for PETRIC 2.0... I suggest to do minor clean-up here, and merge it when all done.
There was a problem hiding this comment.
run(1) is even more likely to fail than __init__ due to lacking GPU... but true, we could add a warning for that too.
PR with commits originally tested in #116, but removed from there pending review.
Fixes #118
This can easily be merged after closing for submissions.