Merge branch 'mr-approval-check' into 'master'

MR approval check for noise changes

See merge request !82

.gitlab-ci.yml

 stages:
   - dist
   - test
-  - gen_cache
-  - update_cache
+  - review
   - docs
   - deploy
 
+# have to specify this so that all jobs execute for all commits
+# including merge requests
+workflow:
+  rules:
+    - if: $CI_MERGE_REQUEST_ID
+    - if: $CI_COMMIT_BRANCH
+
+variables:
+  GIT_STRATEGY: clone
+
 # build the docker image we will use in all the jobs, with all
-# dependencies pre-installed/configured
-.dependencies: &dependencies
+# dependencies pre-installed/configured.
+gwinc/base:
   stage: dist
   variables:
     IMAGE_TAG: $CI_REGISTRY_IMAGE/$CI_JOB_NAME:$CI_COMMIT_REF_NAME
@@ -19,20 +28,18 @@ stages:
       cat <<EOF > Dockerfile
       FROM igwn/base:buster
       RUN apt-get update -qq
-      RUN apt-get -y install --no-install-recommends git python3 python3-yaml python3-scipy python3-matplotlib python3-ipython lalsimulation-python3 python3-pypdf2 python3-h5py
+      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 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
 
-# actually generate the docker image
-images/base:
-  <<: *dependencies
-
-# run the tests and generate the test report on failure
-test:
+# 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:
   stage: test
-  image: $CI_REGISTRY_IMAGE/images/base:$CI_COMMIT_REF_NAME
+  image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
   script:
     - rm -f gwinc_test_report.pdf
     - export MPLBACKEND=agg
@@ -41,14 +48,61 @@ test:
     when: on_failure
     paths:
       - gwinc_test_report.pdf
-    expose_as: 'GWINC test failure report PDF'
+    expose_as: 'noise validation failure report'
+
+# 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:
+  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
+      import gitlab
+      project_id = sys.argv[1]
+      mr_iid = sys.argv[2]
+      # this only works for public repos, otherwise need to specify
+      # private_token=
+      gl = gitlab.Gitlab('https://git.ligo.org')
+      project = gl.projects.get(project_id)
+      mr = project.mergerequests.get(mr_iid)
+      approvals = mr.approvals.get()
+      print(approvals.approved)
+      EOF
+    - echo $CI_MERGE_REQUEST_PROJECT_ID, $CI_MERGE_REQUEST_IID, $CI_MERGE_REQUEST_TARGET_BRANCH_NAME,
+    - approved=$(python3 check_approved.py $CI_MERGE_REQUEST_PROJECT_ID $CI_MERGE_REQUEST_IID )
+    - 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."
+    -         /bin/false
+    -     else
+    -         echo "Reference update did not cause appreciable noise change."
+    -     fi
+    - else
+    -     echo "Merge request approved, reference 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'
 
 # create plots for the canonical IFOs
 ifo:
   stage: docs
-  needs:
-    - test
-  image: $CI_REGISTRY_IMAGE/images/base:$CI_COMMIT_REF_NAME
+  image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
   script:
       - mkdir -p ifo
       - export PYTHONPATH=/inspiral_range
@@ -67,9 +121,7 @@ html:
   stage: docs
   only:
     - master
-  needs:
-    - test
-  image: $CI_REGISTRY_IMAGE/images/base:$CI_COMMIT_REF_NAME
+  image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
   script:
     - rm -rf public
     - apt-get install -y -qq python3-sphinx-rtd-theme
@@ -82,6 +134,8 @@ html:
     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:
@@ -91,7 +145,7 @@ pages:
       artifacts: true
     - job: html
       artifacts: true
-  image: $CI_REGISTRY_IMAGE/images/base:$CI_COMMIT_REF_NAME
+  image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
   script:
     - mv ifo public/
   artifacts:

CONTRIBUTING.md

@@ -42,7 +42,25 @@ 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 budgets differences are
-detected the pipeline will fail, alerting reviewers that a more
-detailed review is required.  Test issues will need to be resolved
-before code can be merged.
+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).
+
+
+## For admins: 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.

gwinc/test/__main__.py

@@ -27,6 +27,7 @@ logging.basicConfig(
     level=os.getenv('LOG_LEVEL', logging.INFO))
 
 
+FREQ = np.logspace(np.log10(5), np.log10(6000), 3000)
 TOLERANCE = 1e-6
 CACHE_LIMIT = 5