search

ecboghiu / inflation

Implementations of the Inflation Technique for Causal Inference.
GNU General Public License v3.0
22 stars 3 forks source link

Add ability to evaluate certificates in distributions #128

Closed apozas closed 6 months ago

apozas commented 7 months ago

This is something that @eliewolfe and I agreed that it would be very handy to have. Given a probability distribution prob, Inflation(L/SD)P.evaluate_certificate(prob) will evaluate the certificate of infeasibility (previously obtained) in prob and output the value.

I checked in some small examples that the function works correctly, the documentation is written, and the website has been changed accordingly.

eliewolfe commented 7 months ago

To avoid duplicate code, I have added helper functions to convert certificates in the form of a dictionary to certificates in the form of sympy expressions and to strings. I also adjusted the optimization_utils to return a dictionary instead of sympy expressions from return_last_certificate=True. This may require example adjusting? Perhaps we should add an example for evaluating a polynomial on a distribution? Test that it works with symbolic distributions and partially_specified distributions? What should happen if compute_marginal returns np.nan?

apozas commented 6 months ago

I fixed the examples so as to run with the new format. Also, I tested that evaluate_polynomial runs without problem on symbolic distributions. Regarding partially specified distributions, the user can already do this by calling certificate_as_probs().subs(whatever_substitutions). I think this is simple enough if one wants to specify only some values for the time being. But, indeed, this and the fact that we have quite some code duplication in InflationLP and InflationSDP regarding the processing of certificates seems to call for a standalone Certificate class.

In addition to the above, I removed the arguments clean, chop_tol and round_decimals from string_from_dict and probs_from_dict. The logic is that these are intermediate functions, and the processing should happen when we process the certificate (i.e., in certificate_as_dict, certificate_as_probs, and certificate_as_string).

apozas commented 6 months ago

After fixing the merge conflicts, from my side, there is green light for merging this PR.