CausalQueries icon indicating copy to clipboard operation
CausalQueries copied to clipboard
verified publisher icon integrated-inferences

Methods optimization

Open gerasy1987 opened this issue 1 year ago • 1 comments

What's new

Significant rewrite of methods and grab functionality:

  • Complete removal of event_probabilities, parameter_mapping, parameter_matrix, parents, parameters_posterior, parameters_prior, posterior_event_probabilities, type distribution and type_prior classes
  • summaries previously produced by grab are now in summary.causal_model; summary.causal_model class now enhances the causal_model class objects with additional summaries
  • print functionality previously contained in print methods for removed classes is now in print.summary.causal_model; the latter takes an optional include = argument that can specify set of summaries to print
  • grab now is just a wrapper for summary.causal_model and corresponding print method
  • updated all places where old grab functionality was used to avoid breaking the package (overall suggestion: we should not use grab internally and just use $ operator; grab can be viewed as user-facing convenience function)

Remaining todo:

  • [ ] Review new functionality and documentation for errors
  • [ ] Update tests (now all outdated tests are commented out and print as empty when testing the package)
  • [ ] See if there need to be additional functionality

Important note:

summary.causal_model() can take additional arguments passed to summaries it calculates via .... That said it applies a specific argument provided (e.g. n_draws = 1000) to all summaries that can take this argument. This is done to allow grab to update the summary on the go, and grab works fine with it. But might not be ideal in general since it can affect multiple summaries when the additional argument is directly passed to summary.causal_model() call.

gerasy1987 avatar Sep 14 '24 19:09 gerasy1987

Amazing

Wow

Looking forward to going over; last edits by TIll and Lily were using inspect rather than grab; will check for compatibility

On Sat 14. Sep 2024 at 21:23, Georgiy (Gosha) Syunyaev < @.***> wrote:

What's new

Significant rewrite of methods and grab functionality:

  • Complete removal of event_probabilities, parameter_mapping, parameter_matrix, parents, parameters_posterior, parameters_prior, posterior_event_probabilities, type distribution and type_prior classes
  • summaries previously produced by grab are now in summary.causal_model; summary.causal_model class now enhances the causal_model class objects with additional summaries
  • print functionality previously contained in print methods for removed classes is now in print.summary.causal_model; the latter takes an optional include = argument that can specify set of summaries to print
  • grab now is just a wrapper for summary.causal_model and corresponding print method
  • updated all places where old grab functionality was used to avoid breaking the package (overall suggestion: we should not use grab internally and just use $ operator; grab can be viewed as user-facing convenience function)

Remaining todo:

  • Review new functionality and documentation for errors
  • Update tests (now all outdated tests are commented out and print as empty when testing the package)
  • See if there need to be additional functionality

Important note:

summary.causal_model() can take additional arguments passed to summaries it calculates via .... That said it applies a specific argument provided (e.g. n_draws = 1000) to all summaries that can take this argument. This is done to allow grab to update the summary on the go, and grab works fine with it. But might not be ideal in general since it can affect multiple summaries when the additional argument is directly passed to summary.causal_model() call.

You can view, comment on, or merge this pull request online at:

https://github.com/integrated-inferences/CausalQueries/pull/357 Commit Summary

File Changes

(61 files https://github.com/integrated-inferences/CausalQueries/pull/357/files)

Patch Links:

  • https://github.com/integrated-inferences/CausalQueries/pull/357.patch
  • https://github.com/integrated-inferences/CausalQueries/pull/357.diff

— Reply to this email directly, view it on GitHub https://github.com/integrated-inferences/CausalQueries/pull/357, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBE57LWX65USITJBMXNYLLZWSELZAVCNFSM6AAAAABOHDDYHKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGUZDMNJXGY4DEMA . You are receiving this because your review was requested.Message ID: @.***>

macartan avatar Sep 14 '24 20:09 macartan