search

pedemonte96 / causaleffect

Python package to compute conditional and non-conditional causal effects.
MIT License
31 stars 5 forks source link

Do not throw an Exception when the effect is not identifiable #2

Open carloscinelli opened 3 years ago

carloscinelli commented 3 years ago

Martí,

Congratulations on this very simple to use package, it seems great so far.

One design suggestion though.

Instead of throwing an Exception when the effect is not identifiable, I believe you should still return a probability object, but with exactly the information you now provide in the Exception message: that the effect is not ID and the hedge.

The reason being that the effect not being identifiable is not an error in itself, it is the correct behavior of the algorithm.

So it is better to return the formal result of the algorithm (non-identifiability and the reason for non-identifiability) in a way that can be stored in an object for further manipulations.

An exception should be used for real errors and exceptions only.

pedemonte96 commented 3 years ago

Dear Carlos,

First of all, thank you for your kind words. This has been a tricky project, and I am pleased my contribution can help the scientific community.

Regarding your suggestion, how would you implement this modification? The fact that the causal effect is not identifiable cannot be encoded in a probability object, because it is not a probabiliy. The thrown exception has to be interpreted as a natural and possible outcome of the algorithm, and not an error.

I understand your concern, but I do not know how to encode the information of non-identifiability into the Probability class.

carloscinelli commented 3 years ago

Hi Marti,

I understand it is not a probability, but it is also not an error.

The way it is currently designed, we would need to handle the exception to capture the useful information you provide (i.e, that the effect is not ID, the hedge etc). So I believe a better solution would be that the function instead returns an output with that information.

(Tikka also made this choice of producing an error in his first causal.effect package, but notice he doesn't do that anymore in the newer packages.)

But in the end of the day, of course, this is a design choice! Just a suggestion.

Best, Carlos