search

cmu-phil / tetrad

Repository for the Tetrad Project, www.phil.cmu.edu/tetrad.
GNU General Public License v2.0
402 stars 110 forks source link

Proposal: Reset Lists of Public-Facing Algorithms, Tests, and Scores. #1008

Closed jdramsey closed 5 years ago

jdramsey commented 5 years ago

Proposal: Set the lists of public-facing algorithms as follows. I have taken Peter’s comment about GLASSO into account. This is a cleanup of an earlier post.

The following may be removed:

For tests:

We can remove Correlation T Test, since it's equivalent to Fisher Z.

For scores:

We may remove SEM BIC Score Deterministic, which needs work.

jdramsey commented 5 years ago

@ps7z @cg09 @espinoj @chirayukong @kvb2univpitt @yuanzhou

Actually, LiNGAM should also be removed for now, until it is fixed, at which point it can be re-added. It is currently broken.

cg09 commented 5 years ago

I will write the manual to include it. If you can't fix it, the manual entry can be deleted.

On Thu, Mar 28, 2019 at 11:40 PM Joseph Ramsey notifications@github.com wrote:

@ps7z https://github.com/ps7z @cg09 https://github.com/cg09 @espinoj https://github.com/espinoj @chirayukong https://github.com/chirayukong @kvb2univpitt https://github.com/kvb2univpitt @yuanzhou https://github.com/yuanzhou

Actually, LiNGAM should also be removed for now, until it is fixed, at which point it can be re-added. It is currently broken.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1008#issuecomment-477855137, or mute the thread https://github.com/notifications/unsubscribe-auth/APmNuXLEu3cpdVjUz7vd0SR4HzlNzMfRks5vbYshgaJpZM4cRj7- .

jdramsey commented 5 years ago

I left out the discrete tests and score. For tests, this add the following (these all seem good):

The only one I'm not familiar with is Probabilistic Test--can someone vouch for that?

For scores, this adds:

Those are both good.

kvb2univpitt commented 5 years ago

To remove Algorithms, Tests, and Scores from the list without delete the code, just remove the annotations edu.cmu.tetrad.annotation.Algorithm, edu.cmu.tetrad.annotation.TestOfIndependence, and edu.cmu.tetrad.annotation.Score, respectively.

chirayukong commented 5 years ago

@jdramsey the probabilistic test is @kvb2univpitt and Greg's work. It uses Bayesian Constraint Inference as a test. It is also used in the RFCI-BSC as the BSC part. Of cause, it is meant to the discrete data only. :-)

jdramsey commented 5 years ago

@chirayukong @kvb2univpitt Is there a very short description of the Probabilistic Test that Clark could put in a manual?

yuanzhou commented 5 years ago

Thanks Joe! Then we no longer need algo descriptions for FASK Concatenated and LiNGAM (until after it's fixed). So the missing descriptions in the HTML manual: FASK, MultiFask.

yuanzhou commented 5 years ago

@jdramsey I'll just disable the FASK Concatenated and LiNGAM in my branch (html-manual-zhou) since I'll be sending a pull request to merge the documentation updates as well as the refactored param descriptions. Then you or one of us can do a thorough cleanup.

jdramsey commented 5 years ago

Clark has done some additional experiments with BPC and FOFC. In the paper with Kummerfeld and Ramsey, we found the two algorithms to be pretty much on par for examples where latents exist. Clark ran some examples where latents do not exist and found that FOFC was able to determine this fact, but BPC was not. Sigh. So the suggestion is that we additionally remove BPC from the interface.

yuanzhou commented 5 years ago

@jdramsey I'll disabled the BPC and send a pull request soon.

yuanzhou commented 5 years ago

1019

jdramsey commented 5 years ago

The only thing left here is the IMaGES adjustment!

yuanzhou commented 5 years ago

@jdramsey The changes that @kvb2univpitt and I made to the IMaGES algorithms will be merged some time today I think. I'm testing the change.

jdramsey commented 5 years ago

Nice!

On Mon, Apr 8, 2019 at 11:10 AM Zhou (Joe) Yuan notifications@github.com wrote:

@jdramsey https://github.com/jdramsey The changes that @kvb2univpitt https://github.com/kvb2univpitt and I made to the IMaGES algorithms will be merged some time today I think. I'm testing the change.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1008#issuecomment-480873090, or mute the thread https://github.com/notifications/unsubscribe-auth/AJZZR1_dLMJezOD8DoSLq_AfauuZvy4iks5ve1vUgaJpZM4cRj7- .

-- Joseph D. Ramsey Special Faculty and Director of Research Computing Department of Philosophy 135 Baker Hall Carnegie Mellon University Pittsburgh, PA 15213

jsph.ramsey@gmail.com Office: (412) 268-8063 http://www.andrew.cmu.edu/user/jdramsey

yuanzhou commented 5 years ago

Addressed IMaGES algorithms via pull request #1047. Now the users will only see IMaGES discrete or continuous based on the input data type. The implementations of these two algorithms stay the same as before to ensure the algorithm integrity.

jdramsey commented 5 years ago

Thanks!

On Mon, Apr 8, 2019 at 2:13 PM Zhou (Joe) Yuan notifications@github.com wrote:

Addressed IMaGES algorithms via pull request #1047 https://github.com/cmu-phil/tetrad/pull/1047. Now the users will only see IMaGES discrete or continuous based on the input data type. The implementations of these two algorithms stay the same as before to ensure the algorithm integrity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1008#issuecomment-480944468, or mute the thread https://github.com/notifications/unsubscribe-auth/AJZZR_FctRhn2J_8pMZAQSs1jOVjd4wzks5ve4a6gaJpZM4cRj7- .

-- Joseph D. Ramsey Special Faculty and Director of Research Computing Department of Philosophy 135 Baker Hall Carnegie Mellon University Pittsburgh, PA 15213

jsph.ramsey@gmail.com Office: (412) 268-8063 http://www.andrew.cmu.edu/user/jdramsey

cg09 commented 5 years ago

Hmm. Why don't we have images-something for search with mixed data types? Images as written isn't appropriate for searches based on regression, but maybe modified?

On Mon, Apr 8, 2019 at 5:46 PM Joseph Ramsey notifications@github.com wrote:

Thanks!

On Mon, Apr 8, 2019 at 2:13 PM Zhou (Joe) Yuan notifications@github.com wrote:

Addressed IMaGES algorithms via pull request #1047 https://github.com/cmu-phil/tetrad/pull/1047. Now the users will only see IMaGES discrete or continuous based on the input data type. The implementations of these two algorithms stay the same as before to ensure the algorithm integrity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1008#issuecomment-480944468, or mute the thread < https://github.com/notifications/unsubscribe-auth/AJZZR_FctRhn2J_8pMZAQSs1jOVjd4wzks5ve4a6gaJpZM4cRj7-

.

-- Joseph D. Ramsey Special Faculty and Director of Research Computing Department of Philosophy 135 Baker Hall Carnegie Mellon University Pittsburgh, PA 15213

jsph.ramsey@gmail.com Office: (412) 268-8063 http://www.andrew.cmu.edu/user/jdramsey

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1008#issuecomment-481020347, or mute the thread https://github.com/notifications/unsubscribe-auth/APmNuUOfia3VD5g4LVyHE_M4v3vnsUV4ks5ve7iYgaJpZM4cRj7- .

jdramsey commented 5 years ago

@cg09 That is less a 'bug fix' and more a 'new feature'. We could make an Images version based on conditional Gaussian, but that would have to be extensively tested before putting it in the interface.

jdramsey commented 5 years ago

@yuanzhou This is all fixed now, yay! Can I close it?

kvb2univpitt commented 5 years ago

@jdramsey I can say "yes, we can close it", since I worked on it. ;) I'll close it.