Skip to content

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Aug 13, 2025

In the Bijectors test suite there are two kinds of AD tests:

  1. Check that the manual implementation of Bijectors.logabsdetjac returns the same numerical result as an AD backend (I think we should ideally like to always use FiniteDifferences, but in some cases we use ForwardDiff). This usually involves a manual call to ForwardDiff.jacobian or similar.

  2. Check that AD backends can differentiate through Bijectors code. This uses the test_ad function.

Generally these fall under the 'interface' / 'AD' test suite respectively.

But there are a couple of stragglers where the test_ad function is being called in the interface tests. This PR gets rid of the exceptions.

This PR also streamlines CI a bit:

  • Removes macOS, I don't think it gives us any extra information over Ubuntu. It also makes it quite hard for other packages to get macOS runners for CI.
  • Don't test min and lts separately (our min compat is 1.10.8 and lts is 1.10.10 so it's really no different).

Copy link
Contributor

Bijectors.jl documentation for PR #405 is available at:
https://TuringLang.github.io/Bijectors.jl/previews/PR405/

Comment on lines -112 to -126
function test_bijector_parameter_gradient(b::Bijectors.Transform, x, y=b(x))
args, re = Functors.functor(b)
recon(k, param) = re(merge(args, NamedTuple{(k,)}((param,))))

# Compute the gradient wrt. one argument at the time.
for (k, v) in pairs(args)
test_ad(p -> sum(transform(recon(k, p), x)), v)
test_ad(p -> logabsdetjac(recon(k, p), x), v)

if Bijectors.isinvertible(b)
test_ad(p -> sum(transform(inv(recon(k, p)), y)), v)
test_ad(p -> logabsdetjac(inv(recon(k, p)), y), v)
end
end
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function was not used anywhere

@penelopeysm penelopeysm requested a review from sunxd3 August 13, 2025 17:18
@penelopeysm
Copy link
Member Author

actually i decided to just fix even more things in #406 so this PR probably doesn't need to be separately reviewed, sorry

@penelopeysm penelopeysm removed the request for review from sunxd3 August 15, 2025 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant