-
Notifications
You must be signed in to change notification settings - Fork 9
v0.10 #305
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
base: main
Are you sure you want to change the base?
v0.10 #305
Conversation
JuliaBUGS.jl documentation for PR #305 is available at: |
Pull Request Test Coverage Report for Build 15303545232Details
💛 - Coveralls |
The scope of `interface.jl` is unclear—it contains a set of miscellaneous functions defined for models, but it's hard to see why they differ from `condition` and `evaluation`. Update: merged `interface.jl` into `model.jl` to balance code per file and keep related code together. --------- Co-authored-by: Xianda Sun <[email protected]> Co-authored-by: Xianda Sun <[email protected]>
I forgot in #307: |
1. `FlattenedGraphNodeData` is renamed to `GraphEvaluationData`. 2. `parameter` field of `BUGSModel` is moved inside `GraphEvaluationData` and named `sorted_parameters` for better understanding. (1) is purely for ease of understanding. (2) is motivated by eliminating an implicit invariant where `parameters` are assumed to be order-consistent with `sorted_nodes` field of `FlattenedGraphNodeData`, not explicit by construction.
Address #252 The old `condition` does two things at once: it creates a `BUGSModel` that contains a subset of the original variables and also mark variables to be conditioned on as `observed`. This was partially motivated by trying to make BUGS' `Gibbs` algorithm looks like Turing.jl's while make use the advantage of the graph. But this is ultimately a badly thought-out interface. This PR implements `condition` and `decondition` to be like DynamicPPL's, i.e., it only sets variables from model parameters to observations. I also took the chance to properly refactored the code to better handle more types of arguments and added some docstrings.
This fix some issues following the file restructuring. It's debatable `source_generation.jl` file should be moved to `model` folder. I left it as is. There is an error message removed. It is tricky - I think expose the raw error to user is probably a better idea, and we can improve the error message if it's required. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
make `/test` mimic `/src` structure-wise. one redundant test is removed, others was just moved around, some files are renamed. CI are split into parallel runs. `experimental` and some hmc tests are failing and I am going to ignore them for now. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Benchmark results on macOS (aarch64)BridgeStan not found at location specified by $BRIDGESTAN environment variable, downloading version 2.6.2 to /Users/runner/.bridgestan/bridgestan-2.6.2 Stan results:
JuliaBUGS Mooncake results:
Benchmark results on Ubuntu (x64)BridgeStan not found at location specified by $BRIDGESTAN environment variable, downloading version 2.6.2 to /home/runner/.bridgestan/bridgestan-2.6.2 Stan results:
JuliaBUGS Mooncake results:
|
This is to address #296. The changes required from current main branch will be substantial.
The current approach is that I'll gradually merge PR into
v0.10
branch. And eventually make a non-squash merge.I am not entirely clear about the code change that are required, so I might flip-flop on ideas and design, but I'll make the motivation and changes clear.
I will likely to move fast and not request a lot of reviews, and sometimes code changes will break some tests, but their corresponding test will pass. And before merging , all tests will pass.