causal-learn
causal-learn copied to clipboard
Inconsistent return types
One general problem we're having in comparing algorithms to one another is that algorithms that basically should return a GeneralGraph return it in different ways. So for instance, the way to get a graph from PC is different from the way one gets it from FCI or from GES. Is it possible to make this uniform with the same syntax?
Basically one should be able to write something like this (I think):
G = search_algorithm(data, ...)
uniformly, or perhaps
G = search_algorithm(data,...).G
maybe, but uniformly for all algorithms, it seems.
Yeah, I totally agree with you. This issue has been listed in our issue tracking doc and we are currently working on it. Let me share the doc with you offline : )
Yes, I totally agree.
The problem is: our codebase in not designed in a OOP manner. The algorithms should extend the same base class, and the base class should have a unifying API, i.e. .run(), which return a GeneralGraph.
The same thing also happens to Independence Test --- previously these are just functions and not organized well.
I plan to initiate some refactors in the near future.
Currently, I am pushing to adding more tests into our code bases to guarantee the code correctness first. Without good tests, it's very hard to guarantee the code correctness when doing some huge code refactor.
Let me know what you think and I am happy to discuss. :)
That seems very sensible to me. OOP is definitely a good solution to this problem.
Also, I agree that it makes a lot more sense to do refactoring when there are good tests. Even a few well-placed tests can save a lot of headache, but more tests are better.
Totally agree!! :) More tests, the better! (Learned this lesson in a hard way lol)