Add GitHub Actions CI workflow

[sethaxen]

This PR adds a GitHub Actions for CI. The goal is ultimately to replace Travis, though I recommend keeping both in parallel for a little while until we are sure we are happy with this.

cc @simeonschaub

[sethaxen]

Once everything is working well (including code coverage) for release versions, I'll add another workflow that allows failure for nightly, as well as a badge.

[simeonschaub]

Awesome, thanks for doing this! Looks great!

[sethaxen]

Coveralls appears to be poorly supported by Coverage outside of Travis. Can we switch to codecov?

[oxinabox]

Coveralls appears to be poorly supported by Coverage outside of Travis. Can we switch to codecov?

Sure, i can never remember which is which.
I know i like one slightly better than the other, but I don't know what it is.

[nickrobinson251]

Thanks for this!!

I recommend keeping both in parallel for a little

👍

Before we turn off Travis, we need to figure out how to have GA run CI nightly and notify us of failures

i.e. the GA version of the current Travis Cron job and

notifications:
  email:
    recipients:
      - nightly-rse@invenia.ca
    on_success: never
    on_failure: always
    if: type = cron

[sethaxen]

Before we turn off Travis, we need to figure out how to have GA run CI nightly and notify us of failures

i.e. the GA version of the current Travis Cron job and

That shouldn't be hard to do. Is the cron job only run on master or also for PRs? Where is the cron job specified?

notifications:
  email:
    recipients:
      - nightly-rse@invenia.ca
    on_success: never
    on_failure: always
    if: type = cron

This is trickier, as GA's notification options are more limited. I'll see if I can figure it out with a 3rd party action.

[sethaxen]

This is trickier, as GA's notification options are more limited. I'll see if I can figure it out with a 3rd party action.

Something like this seems to be the recommended approach: https://github.saobby.my.eu.orgmunity/t/no-email-notification-on-schedule-cron-job-failure/119638/2. i.e. create a new e-mail address specifically for e-mailing these notifications, and add its password to secrets. This is really inconvenient.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@f550f8d). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #309   +/-   ##
=========================================
  Coverage          ?   97.15%           
=========================================
  Files             ?       16           
  Lines             ?      880           
  Branches          ?        0           
=========================================
  Hits              ?      855           
  Misses            ?       25           
  Partials          ?        0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f550f8d...b6810f7. Read the comment docs.

[nickrobinson251]

Something like this seems to be the recommended approach: ...

Ooof. Thanks for looking into it! Maybe let's leave it for now? ...I wonder if we can just keeping travis for the nightly CI run only? Don't think we need to solve it in this PR. :)

[sethaxen]

Okay, this PR is ready for review. A few notes:

• All 32-bit builds fail. This is because the rrule test for logdet is too stringent. But this is the default for rrule_test, which sets atol and rtol to 1e-9. Should this be relaxed in ChainRulesTestUtils.jl to use something more like the isapprox defaults, which account for the precision of the number type?
• Julia nightly versions currently fail due to imprecision of BLAS.nrm2
• codecov and coveralls are both set up. But IMO codecov is nicer with github actions, since it automatically combines the code coverage into one metric instead of giving one for every job. Potentially we could completely drop coveralls for github actions, just using it for Travis.
• Julia nightly versions currently do not contribute to code coverage.

[oxinabox]

@mattBrzezinski can i leave it to you to see this merged?
You know GitHub Actions better

[mattBrzezinski]

The changes make sense, not sure how the owners want to handle the failures of the builds.

[sethaxen]

There are two different failures:

• logdet/logabsdet tests: the result looks right, but perhaps it's hard for FiniteDifferences to get high precision consistently for those rules, so I propose to slightly increase the tolerance for those tests
• logabsgamma test: this failure seems to have been introduced in Tweak Zero to vec FiniteDifferences.jl#115. The offending line is

  ChainRules.jl/test/rulesets/packages/SpecialFunctions.jl

  Line 53 in 4274352

  Δz = Composite{Tuple{Float64, DoesNotExist}}(randn(), Zero())

julia> Δz = Composite{Tuple{Float64, DoesNotExist}}(1.0, Zero())
Composite{Tuple{Float64, DoesNotExist}}(1.0, Zero())
FiniteDifferences v0.11.3 changed how `to_vec` processes this differential, which causes the failure:
julia> to_vec(Δz)[1] # v0.11.2
2-element Vector{Float64}:
 -0.06132606674859018
  0.0

julia> to_vec(Δz)[1] # v0.11.3
1-element Vector{Float64}:
 -0.06132606674859018

@willtebbutt can you comment on whether this is an FD bug or whether we need to use a different differential for the test here?

[oxinabox]

logdet/logabsdet tests: the result looks right, but perhaps it's hard for FiniteDifferences to get high precision consistently for those rules, so I propose to slightly increase the tolerance for those tests

Possibly we need to use a matrix with larger values to avoid issues near zero?
But yes, fine to increase atol and rtol, there is an argument for it to pass in to the testers

[willtebbutt]

@willtebbutt can you comment on whether this is an FD bug or whether we need to use a different differential for the test here?

The reported new behaviour is correct -- the previous behaviour was incorrect (to_vec(::Zero) should be an empty vector).

Since we're still treating Integers as differentiable things with to_vec(1) = [1], you should replace Zero() with randn() I think.