Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Assorted Problems #203

Merged
merged 4 commits into from
Jul 26, 2024
Merged

Fix Assorted Problems #203

merged 4 commits into from
Jul 26, 2024

Conversation

willtebbutt
Copy link
Member

Resolves a couple of small problems needed to make #202 run.

@willtebbutt
Copy link
Member Author

willtebbutt commented Jul 26, 2024

@sunxd3 could you confirm that, if you check out to this branch, your example runs?

@sunxd3
Copy link
Collaborator

sunxd3 commented Jul 26, 2024

With test_rule, it fails with msg

Test Failed at /Users/xiandasun/.julia/packages/Tapir/mkYSZ/src/test_utils.jl:268
  Expression: has_equal_data(x_primal, map(primal, x_x̄_rule))

ERROR: There was an error during testing

But LogDensityProblems.logdensity_and_gradient will return without error.

Not sure what to make of this. Thanks a lot for looking into it!

@willtebbutt
Copy link
Member Author

willtebbutt commented Jul 26, 2024

Great stuff. Yeah, I have the same thing. I suspect that there's something going on with some non-determinism in the Dicts or something which means that certain values can't be compared properly. The rest of the tests seem to pass without issue, so I'm not worried.

@willtebbutt
Copy link
Member Author

I'll merge this in and tag a release when CI passes.

@sunxd3
Copy link
Collaborator

sunxd3 commented Jul 26, 2024

Lovely, thanks Will

Copy link
Contributor

Performance Ratio:
Warning: results are very approximate!

┌────────────────────────────┬────────┬─────────┬─────────────┬─────────┐
│                      Label │  Tapir │  Zygote │ ReverseDiff │  Enzyme │
│                     String │ String │  String │      String │  String │
├────────────────────────────┼────────┼─────────┼─────────────┼─────────┤
│                        sum │   40.6 │   0.439 │         2.9 │   0.628 │
│                       _sum │   6.95 │   488.0 │        27.0 │   0.127 │
│                   kron_sum │   80.9 │    3.45 │       206.0 │    23.3 │
│              kron_view_sum │   93.6 │    11.0 │       239.0 │     8.2 │
│      naive_map_sin_cos_exp │   4.13 │ missing │        9.01 │    2.78 │
│            map_sin_cos_exp │    4.9 │    1.76 │        7.65 │    3.44 │
│      broadcast_sin_cos_exp │    4.6 │    2.63 │        1.68 │    2.86 │
│                 simple_mlp │   9.38 │    3.26 │        12.1 │    3.34 │
│                     gp_lml │   15.7 │     4.4 │     missing │ missing │
│ turing_broadcast_benchmark │   8.57 │ missing │        26.3 │ missing │
└────────────────────────────┴────────┴─────────┴─────────────┴─────────┘

@willtebbutt willtebbutt merged commit bb95e0f into main Jul 26, 2024
17 checks passed
@willtebbutt willtebbutt deleted the wct/fix-some-issues branch July 26, 2024 16: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.

2 participants