diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4ca55495087b2781d111783a86aa58c9275d909c..55a6077ddedf83f8b5b7f0424fa03a57cbf6b867 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -2,7 +2,6 @@ stages:
- dist
- test
- review
- - docs
- deploy
# have to specify this so that all jobs execute for all commits
@@ -28,44 +27,42 @@ gwinc/base:
cat <<EOF > Dockerfile
FROM igwn/base:buster
RUN apt-get update -qq
- RUN apt-get -y install --no-install-recommends git gitlab-cli python3 python3-yaml python3-scipy python3-matplotlib python3-ipython lalsimulation-python3 python3-pypdf2 python3-h5py
+ RUN apt-get -y install --no-install-recommends git python3-gitlab python3 python3-yaml python3-scipy python3-matplotlib python3-ipython python3-lalsimulation python3-pypdf2 python3-h5py
RUN git clone https://gitlab-ci-token:ci_token@git.ligo.org/gwinc/inspiral_range.git
EOF
- docker build -t $IMAGE_TAG .
- docker push $IMAGE_TAG
-# validate that the noises haven't changed relative to the reference
-# (the reference itself could have been updated, though, see
-# check_approval job)
-validate_noise:
+# create plots for the canonical IFOs
+generate_budgets:
stage: test
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- - rm -f gwinc_test_report.pdf
- - export MPLBACKEND=agg
- - python3 -m gwinc.test -r gwinc_test_report.pdf
+ - mkdir -p ifo
+ - export PYTHONPATH=/inspiral_range
+ - for ifo in $(python3 -c "import gwinc; print(' '.join(gwinc.IFOS))"); do
+ - python3 -m gwinc $ifo -s ifo/$ifo.png
+ - python3 -m gwinc $ifo -s ifo/$ifo.h5
+ - done
+ - python3 -m gwinc.ifo -s ifo/all_compare.png
artifacts:
- when: on_failure
+ when: always
paths:
- - gwinc_test_report.pdf
- expose_as: 'noise validation failure report'
+ - ifo
-# this is a special job intended to run only for merge requests where
-# the test reference hash file has been updated, indicating that there
-# has been a noise change. if the merge request has not yet been
-# approved, generate a report of noise changes relative to the target
-# branch and present that to the reviewers. if the merge request is
-# approved, re-run this job, which will succeed if the MR is approved.
-check_approval:
+# this is a special job intended to run only for merge requests.
+# budgets are compared against those from the target branch. if the
+# merge request has not yet been approved and noise changes are found,
+# the job will fail. once the merge request is approved the job can
+# be re-run, at which point the pipeline should succeed allowing the
+# merge to be merged.
+noise_change_approval:
stage: review
rules:
# - if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"'
- if: $CI_MERGE_REQUEST_ID
- changes:
- - gwinc/test/ref_hash
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- - echo "NOISE REFERENCE CHANGE, checking approval..."
- |
cat <<EOF > check_approved.py
import sys
@@ -85,42 +82,31 @@ check_approval:
- if [[ $approved != True ]] ; then
- old_hash=$(git cat-file -p origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME:gwinc/test/ref_hash)
- if ! python3 -m gwinc.test --git-ref $old_hash -r gwinc_test_report.pdf ; then
- - echo "APPROVAL REQUIRED TO MERGE THIS BRANCH."
+ - echo "NOISE CHANGES RELATIVE TO $CI_MERGE_REQUEST_TARGET_BRANCH_NAME."
+ - echo "Approval required to merge this branch."
- /bin/false
- else
- - echo "Reference update did not cause appreciable noise change."
+ - echo "No noise changes detected."
- fi
- else
- - echo "Merge request approved, reference change accepted."
+ - echo "Merge request approved, noise change accepted."
- fi
artifacts:
when: on_failure
paths:
- gwinc_test_report.pdf
- expose_as: 'noise changes relative to target branch head APPROVAL REQUIRED TO MERGE'
+ expose_as: 'noise changes relative to target branch'
-# create plots for the canonical IFOs
-ifo:
- stage: docs
- image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
- script:
- - mkdir -p ifo
- - export PYTHONPATH=/inspiral_range
- - for ifo in $(python3 -c "import gwinc; print(' '.join(gwinc.IFOS))"); do
- - python3 -m gwinc $ifo -s ifo/$ifo.png
- - python3 -m gwinc $ifo -s ifo/$ifo.h5
- - done
- - python3 -m gwinc.ifo -s ifo/all_compare.png
- artifacts:
- when: always
- paths:
- - ifo
-
-# generate the html doc web pages
-html:
- stage: docs
+# generate the html doc web pages. the "pages" job has special
+# meaning, as it's "public" artifact becomes the directory served
+# through gitlab static pages
+pages:
+ stage: deploy
only:
- master
+ needs:
+ - job: generate_budgets
+ artifacts: true
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- rm -rf public
@@ -129,24 +115,6 @@ html:
- make html
- cd ..
- mv ./build/sphinx/html public
- artifacts:
- when: always
- paths:
- - public
-
-# the "pages" job has special meaning, as it's "public" artifact
-# becomes the directory served through gitlab static pages
-pages:
- stage: deploy
- only:
- - master
- needs:
- - job: ifo
- artifacts: true
- - job: html
- artifacts: true
- image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
- script:
- mv ifo public/
artifacts:
when: always
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index f7e67fb6d0fba7499fff7a17c080fb2e86cd6fac..815596463f99a0e6f10f766969cf7eb1a5086de3 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -16,11 +16,9 @@ as possible, and provide a complete, descriptive log of the changes
legibility.
`pygwinc` comes with a validation command that can compare budgets
-between different versions of the code and produce a graphical report.
-`pygwinc` stores a reference to the git commit that is considered the
-latest good reference for the budget calculation functions, and the
-validation is run against that reference by default. The command can
-be run with:
+from the current code against those produced from different versions
+in git (by default it compares against the current HEAD). The command
+can be run with:
```shell
$ python3 -m gwinc.test
```
@@ -30,37 +28,22 @@ arbitrary commit using the '--git-ref' option. Traces for referenced
commits are cached, which speeds up subsequent comparison runs
significantly.
-Commits for code changes that modify budget calculations should be
-followed up updates to test reference. This can be done with the
-'--update-ref' option to the test command:
-```shell
-$ python3 -m gwinc.test --update-ref
-```
-If no specific reference is provided, a pointer to the last commit
-will be made. *Note: reference updates need to be made in a separate
-commit after the commits that change the noise calculations* (git
-commits can't have foreknowledge of their own commit hash).
-
-Once you submit your merge request, an automated pipeline will run the
-test command to validate your changes. If there are budgets
-differences but the reference has not been updated, the pipeline will
-fail and indicate that a reference update is required. Updates to the
-reference will also cause a validation pipeline failure, but these
-failures can be resolved through reviewer approval of the changes (see
-below).
+Once you submit your merge request a special CI job will determine if
+there are budgets differences between your code and the master branch.
+If there are, explicit approval from reviewers will be required before
+your changes can be merged (see "approving noise" below).
-## For admins: approving noise curve changes
+## For reviewers: approving noise curve changes
As discussed above, merge requests that generate noise changes will
-cause pipeline failures. If the MR properly includes a reference
-update, then the failure should only be in the approval check. A
-report will be generated comparing all noise changes against the
-target branch (usually 'master'). See the 'View exposed artifacts'
-menu item in the pipeline report. Once you have reviewed the report
-and the code, and understand and accept the noise changes, click the
-'Approve' button in the MR. Once sufficient approval has been given,
-re-run the `review:check_approval` pipeline job, which should now pick
-up that approval has been given and allow the pipeline to succeed.
-Once the pipeline succeeds the merge can be enacted. Click the
-'Merge' button to finally merge the code.
+cause a pipeline failure in the `review:noise_change_approval` CI job.
+The job will generate a report comparing the new noise traces against
+those from master, which can be found under the 'View exposed
+artifacts' menu item in the pipeline report. Once you have reviewed
+the report and the code, and understand and accept the noise changes,
+click the 'Approve' button in the MR. Once sufficient approval has
+been given, `review:noise_change_approval` job can be re-run, which
+should now pick up that approval has been given and allow the pipeline
+to succeed. Once the pipeline succeeds the merge request can be
+merged. Click the 'Merge' button to finally merge the code.
diff --git a/gwinc/test/__main__.py b/gwinc/test/__main__.py
index 532030e46517ee9300be9edd839406d3ca8a7f3c..9f2dfa01d5d904359166875f6cd3ed37c57658d4 100644
--- a/gwinc/test/__main__.py
+++ b/gwinc/test/__main__.py
@@ -42,28 +42,10 @@ def git_ref_resolve_hash(git_ref):
return subprocess.run(
['git', 'show', '-s', '--format=format:%H', git_ref],
capture_output=True, universal_newlines=True,
+ check=True,
).stdout
- except subprocess.CalledProcessError:
- return None
-
-
-def write_ref_hash(ref_hash):
- """Write ref hash to reference file
-
- """
- with open(test_path('ref_hash'), 'w') as f:
- f.write('{}\n'.format(ref_hash))
-
-
-def load_ref_hash():
- """Load the current reference git hash.
-
- """
- try:
- with open(test_path('ref_hash')) as f:
- return f.read().strip()
- except IOError:
- return None
+ except subprocess.CalledProcessError as e:
+ logging.error(e.stderr.split('\n')[0])
def prune_cache_dir():
@@ -79,7 +61,7 @@ def prune_cache_dir():
)[CACHE_LIMIT:]
if not expired_paths:
return
- logging.info("pruning {} old caches...".format(len(expired_paths)))
+ logging.info("pruning {} old cache...".format(len(expired_paths)))
for path in expired_paths:
logging.debug("pruning {}...".format(path))
shutil.rmtree(path)
@@ -248,12 +230,9 @@ def main():
'--skip', '-k', metavar='NOISE', action='append',
help='traces to skip in comparison (multiple may be specified)')
parser.add_argument(
- '--git-ref', '-g', metavar='HASH',
+ '--git-ref', '-g', metavar='HASH', default='HEAD',
help='specify git ref to compare against')
rgroup = parser.add_mutually_exclusive_group()
- rgroup.add_argument(
- '--update-ref', '-u', metavar='HASH', nargs='?', const='HEAD',
- help="update the stored reference git hash to HASH (or 'HEAD' if not specified) and exit")
rgroup.add_argument(
'--plot', '-p', action='store_true',
help='plot differences')
@@ -265,46 +244,11 @@ def main():
help='specific ifos to test (default all)')
args = parser.parse_args()
- # get the current hash of HEAD
- head_hash = git_ref_resolve_hash('HEAD')
- if not head_hash:
- logging.warning("could not determine git HEAD hash.")
-
- # update the reference if specified
- if args.update_ref:
- if args.update_ref == 'HEAD':
- if not head_hash:
- sys.exit("Could not update reference to head.")
- logging.info("updating reference to HEAD...")
- ref_hash = head_hash
- else:
- ref_hash = git_ref_resolve_hash(args.update_ref)
- logging.info("updating reference git hash: {}".format(ref_hash))
- write_ref_hash(ref_hash)
- sys.exit()
-
# get the reference hash
- if args.git_ref:
- ref_hash = git_ref_resolve_hash(args.git_ref)
- else:
- ref_hash = load_ref_hash()
- if not ref_hash:
- pass
- try:
- with open(test_path('ref_hash')) as f:
- ref_hash = f.read().strip()
- except IOError:
- logging.warning("could not open reference")
- sys.exit("Unspecified reference git hash, could not run test.")
-
- logging.info("head hash: {}".format(head_hash))
- logging.info("ref hash: {}".format(ref_hash))
-
- # don't bother test if hashes match
- if ref_hash == head_hash:
- logging.info("HEAD matches reference, not bothering to calculate.")
- logging.info("Use --git-ref to compare against an arbitrary git commit.")
- sys.exit()
+ ref_hash = git_ref_resolve_hash(args.git_ref)
+ if not ref_hash:
+ sys.exit("Could not resolve reference, could not run test.")
+ logging.info("ref hash: {}".format(ref_hash))
# load the cache
cache_path = test_path('cache', ref_hash)