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

Remove BangBang.possible #579

Merged
merged 11 commits into from
Apr 18, 2024
Merged

Remove BangBang.possible #579

merged 11 commits into from
Apr 18, 2024

Conversation

sunxd3
Copy link
Member

@sunxd3 sunxd3 commented Feb 16, 2024

In light of JuliaFolds2/BangBang.jl@e679bae, we can removed the code as they are implemented in BangBang.

The changes has not been released, so should verify afterward. And this PR should not be merged right now.

related: fix #530

@sunxd3
Copy link
Member Author

sunxd3 commented Feb 16, 2024

I don't understand why Document-CI is failing, FLoop is not in BangBang's deps.

@yebai
Copy link
Member

yebai commented Feb 28, 2024

I don't understand why Document-CI is failing, FLoop is not in BangBang's deps.

Maybe Floop is in the deps on another package? I think Julia's package manager has a mechanism to figure which package requires a specific deps.

@devmotion
Copy link
Member

I don't understand why Document-CI is failing, FLoop is not in BangBang's deps.

MLUtils is a docs dependency and depends on FLoops - however FLoops does not support BangBang >= 0.4 currently.

@devmotion
Copy link
Member

MLUtils is used in a single example:

using MLUtils
function cross_val(
dataset::AbstractVector{<:Real};
nfolds::Int=5,
nsamples::Int=1_000,
rng::Random.AbstractRNG=Random.default_rng(),
)
# Initialize `loss` in a way such that the loop below does not change its type
model = gdemo(1) | (x=[first(dataset)],)
loss = zero(logjoint(model, rand(rng, model)))
for (train, validation) in kfolds(dataset, nfolds)
# First, we train the model on the training set, i.e., we obtain samples from the posterior.
# For normally-distributed data, the posterior can be computed in closed form.
# For general models, however, typically samples will be generated using MCMC with Turing.
posterior = Normal(mean(train), 1)
samples = rand(rng, posterior, nsamples)
# Evaluation on the validation set.
validation_model = gdemo(length(validation)) | (x=validation,)
loss += sum(samples) do sample
logjoint(validation_model, (μ=sample,))
end
end
return loss
end
cross_val(dataset)

Since it's a quite heavy dependency, I wonder if we could avoid it.

docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
docs/src/tutorials/prob-interface.md Outdated Show resolved Hide resolved
sunxd3 and others added 2 commits February 29, 2024 13:18
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sunxd3
Copy link
Member Author

sunxd3 commented Mar 1, 2024

Okay, the situation is a bit troublesome.
BangBang v0.4 moved to Accesssors, so all the Setfield related code are erroring out.
Now, either we led some effort in backporting the possible fix to v0.3, or ourselves move to Accessors too.
The interface diff between Setfield and Accessors should be minimal for our purpose, though.

@yebai
Copy link
Member

yebai commented Mar 1, 2024

Now, either we led some effort in backporting the possible fix to v0.3, or ourselves move to Accessors too.

Let's move to Accessors too. @sunxd3, can you create a PR for that next week?

@yebai yebai marked this pull request as ready for review April 18, 2024 21:51
@yebai yebai enabled auto-merge April 18, 2024 22:02
@coveralls
Copy link

coveralls commented Apr 18, 2024

Pull Request Test Coverage Report for Build 8745141284

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 81.821%

Totals Coverage Status
Change from base Build 8745047931: -0.03%
Covered Lines: 2615
Relevant Lines: 3196

💛 - Coveralls

@yebai yebai added this pull request to the merge queue Apr 18, 2024
Merged via the queue into master with commit 0c40cbd Apr 18, 2024
11 of 12 checks passed
@yebai yebai deleted the sunxd/remove_bangbang.hack branch April 18, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants