ReactiveBayes / RxInfer.jl

Julia package for automated Bayesian inference on a factor graph with reactive message passing
MIT License
265 stars 23 forks source link

Merge `rxinference`and `inference` functions into `infer` #190

Closed albertpod closed 9 months ago

albertpod commented 10 months ago

This PR addresses issue #181 and marks significant update to the RxInfer, wherein we merge the functionalities of inference and rxinference into a single, "umbrella" function: infer. This change simplifies the user interface by providing a unified method for conducting both static and streaming probabilistic inference.

The decision to use infer for static or streaming datasets is now determined by the presence of the autoupdates keyword. This keyword is crucial in controlling how updated posteriors are transformed into priors for the following observations, making it specifically applicable and essential for streaming data (real-world) scenarios.

Key Changes:

  1. Unified Inference Functionality: The infer function consolidates all features previously divided between inference and rxinference. The original functions have been internally renamed to __inference and __rxinference and are no longer exported. This unified approach enables infer to support both batch/static and streaming/online applications.

  2. Updated Documentation: Comprehensive updates have been made to the documentation to reflect the merging of inference and rxinference. This includes a detailed and clear docstring for the infer function, ensuring ease of use and understanding.

  3. Updated Examples: All examples across the package have been updated to utilize the new infer function, demonstrating its application in various contexts.

  4. Deprecation Notices: Appropriate deprecation notices have been added for the inference and rxinference functions, guiding users towards the new unified approach. (I haven't managed to make @deprecate inference(; kwargs...) infer(kwargs...) work, seems deprecate macro isn't good for keyword arguments)

bvdmitri commented 10 months ago

I guess we can make it mandatory and allow missing as an input in case someone comes up with a scenario where it is not mandatory.

review-notebook-app[bot] commented 10 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov[bot] commented 10 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (792c6e3) 80.31% compared to head (a805ce9) 80.31%.

Files Patch % Lines
src/inference.jl 88.88% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #190 +/- ## ========================================== - Coverage 80.31% 80.31% -0.01% ========================================== Files 11 11 Lines 1285 1290 +5 ========================================== + Hits 1032 1036 +4 - Misses 253 254 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

albertpod commented 10 months ago

I recommend not halting this PR for long @bvdmitri