Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Knowledge and some other fixes. #1318

Merged
merged 4 commits into from
Jan 25, 2021
Merged

Knowledge and some other fixes. #1318

merged 4 commits into from
Jan 25, 2021

Conversation

jdramsey
Copy link
Collaborator

@jdramsey jdramsey commented Jan 7, 2021

Mainly fixing a knowledge bug but also fixing issues in IndTestFisherZ and SemBicScore--these were using testwise deletion when they didn't need to. To pull over the SEM BIC code from another branch, I also needed to pull over a modest revision to the FGES code, where I shore up some of the reasoning.

…Z and SemBicScore--these were using testwise deletion when they didn't need to. To pull over the SEM BIC code from another branch, I also needed to pull over a modest revision to the FGES code, where I shore up some of the reasoning.
@jdramsey jdramsey requested a review from espinoj January 7, 2021 20:28
@jdramsey
Copy link
Collaborator Author

jdramsey commented Jan 7, 2021

Tests pass. The problem with Knowledge was that it wasn't forbidding all necessary backward edges. The problem with Fisher Z and SEM BIC was that if you gave them large datasets they would try to do the slow testwise deletion, even then there were no missing values. The changes for FGES were needed because of the changes to SEM BIC. Basically, a turning step was added to FGES a la PCALG's GES, which is being left off for now. Also, extra checks are added to the preconditions for some of the rules to make sure the rules are applied correctly in every case. Corresponding adjustments were made to Params and the manual.

…ion editor, where if you click the simulate bug twice you don't get a news simulation. You have to select a new model type. Fixed this. Affected several files.

For FGES, decided to add the turning and tdepth parameters. Adjusted the manual.
@jdramsey
Copy link
Collaborator Author

jdramsey commented Jan 8, 2021

That's it. These bugs fixed:

  • Knowledge.
  • Fisher Z and Sem Bic with datasets that don't contain missing values.
  • The interface "Simulate" bug.
  • Some incidental changes to FGES (the algorithm hasn't changed).

@espinoj
Copy link
Collaborator

espinoj commented Jan 8, 2021 via email

@kvb2univpitt kvb2univpitt requested review from kvb2univpitt and removed request for espinoj January 23, 2021 15:02
Copy link
Collaborator

@kvb2univpitt kvb2univpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested knowledge, and it seems to be working fine. Ran data with and without missing data on FisherZ and SemBIC didn't seem to break anything.

@jdramsey
Copy link
Collaborator Author

jdramsey commented Jan 25, 2021 via email

@jdramsey jdramsey merged commit ce4d3f3 into development Jan 25, 2021
@jdramsey
Copy link
Collaborator Author

@espinoj Hey Jeremy--Kevin reviewed this pull request--would you have to post it (as a bug fix branch)?

Thanks,

Joe

@cg09 cg09 mentioned this pull request Feb 22, 2021
@jdramsey jdramsey deleted the joe-knowledge-fix branch April 18, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants