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

joe_work_2024_6-13 #1800

Merged
merged 162 commits into from
Aug 11, 2024
Merged

joe_work_2024_6-13 #1800

merged 162 commits into from
Aug 11, 2024

Conversation

jdramsey
Copy link
Collaborator

Several changes for LV-Lite and related code.

jdramsey added 30 commits June 14, 2024 06:23
…rch algorithms

Added classes for calculating various statistics related to graph elements such as arrows, colliders, edges within colliders, and unshielded colliders in the estimated graph. The methods obtain counts and represent them as ratios for comparison. Also updated depth control in search algorithms LvLite and LvLiteDsepFriendly to provide more flexibility in configuring search depth. Minor code comment corrections and adjustments in certain classes were also made.
Modified the GridSearch module to provide a graph viewing functionality. Added a new tab for viewing graphs, which allows the user to select the simulation, algorithm, and graph index for viewing the graph from the comparison table. Updated the JComboBoxes to handle changes in selection and reflect the respective graph on the workbench panel. Removed unused codes and fixed the formatting and serialization of some parameters.
Method documentation for setting depth in certain search classes has been added. Additionally, the statistics calculation function in the Comparison class has been refactored to accept an additional `AlgorithmWrapper` argument, leading to changes in several method calls and the `AlgorithmTask` constructor. The `AlgorithmTask` class now also includes an `algorithmWrappers` field to support this change.
The commit includes updating versions of the `maven-compiler-plugin` and `maven-shade-plugin` and changes the `java.version` property in several pom.xml files. Alongside these updates, it also includes adding a significant amount of JavaDoc comments in multiple class files to improve code readability and understanding. The commit also removes an unused variable in the `FaskForbiddenGraphModel.java`.
The commit makes all JTextArea, JButton, JComboBox, JScrollPane, and JTabbedPane components in GridSearchEditor transient. This change is to avoid potential issues in serialization of GUI components. Additionally, this refactor moves the creation code for the 'Edit Utilities' button into a separate helper method. This method is called in appropriate places, improving code readability and facilitating reuse.
The commit mainly directs verbose output to System.out instead of a PrintStream object. This change is reflected across different modules like Fas, FgesMb, SpFci, and more. Along with this, validation has been added to check whether parameters assigned are serializable before setting them in the Parameters class. This aims to avoid errors while saving the parameters object as a field. Unnecessary parameter "printStream" was also removed from Params and its usage in various classes like GridSearchModel and Comparison has been updated.
Removed unused MagSemBicScore from the edu.cmu.tetrad.algcomparison.score package. This file was not being used or referenced in the codebase. Also, added new scoring implementation files MagDgBicScore and MagCgBicScore to edu.cmu.tetrad.search.work_in_progress package. Made minor adjustments in GridSearchModel for print streams.
This commit includes refactoring of the likelihood tests in 'IndTestFisherZ', 'IndTestConditionalGaussianLrt' and 'IndTestDegenerateGaussianLrt' files. This includes changes in the way rows are handled. Additionally, several unnecessary loops and verbose logs have been removed to make the code more efficient. The datatype tests in 'tetrad-lib.properties' have been updated and some minor modifications in 'MarkovCheck', 'BossPag', 'MagCgScore' and other files to improve performance.
The number of bins in the histogram has been reduced from 20 to 10 for compression. Moreover, an optimization has been added to the Markov check algorithm to remove extraneous variables more efficiently. The check for variable removal will now be performed in the specific sections of the algorithm where it is necessary.
The call to MyWatchedProcess() was removed from the PathsAction.java in the tetrad-gui module. This was an unnecessary call and it was not being used anywhere in the application. This change should not affect any functionalities or features.
The Meek rules now include a provision to prevent cycles in the graph. The option to prevent cycles can be toggled on by the user. A check has been added to ensure that new cycles are not created while adding arbitrary unshielded colliders. The colliderAllowed method has been updated to include a check on graph ancestors. This prevents colliders from being activated where they would introduce cycles, thus maintaining the acyclic nature of the graph.
The meekPreventCycles flag has been added and set to true by default. This will prevent the creation of cycles in the graph by adding arbitrary unshielded colliders. Users have the option to turn this setting off to avoid cycle prevention, which allows the PC algorithm to always output a CPDAG.
The refactoring of the MeekRules class has resulted in better documentation and more readable code structure. This includes more detailed comments for class attributes and essential methods, minor code reordering for improved readability, and slight adjustments to variable explanations to enhance clarity. Changes should help future developers understand the functionality and purpose of this class better.
Revised logging to be controlled by a flag and added explanatory comments for increased readability. In MeekRules, logger is used to prevent cycles while orienting edges. In PcCommon, the method 'colliderAllowed' was moved to a more appropriate location and all the logging in the file was modified to use the updated log method.
The addition of directed edges in the MeekRules class has been relocated within the code. It now falls after the verbose conditions. This ensures that all the necessary conditions are checked before modification of the graph structure. It forms part of the process of preventing new cycles in the graph.
Added an option in the Meek rules to prevent cycles when orienting the graph structure. This was accomplished by including a 'meekPreventCycles' flag to the relevant search classes, which instructs them to avoid constructing cycles during the search process. This feature was also integrated into the corresponding test cases.
The parameter "meekPreventCycles" has been replaced with "guaranteeCpdag" in several classes to better describe its functionality. This change is aimed to ensure the output to be a Consensus Partially Directed Acyclic Graph (CPDAG) in the search. The commit also involves updates in corresponding test cases and documentations.
Simplified the markov check in MarkovCheckKolmogorovSmirnoffP and MarkovCheckAndersonDarlingP by removing unnecessary loop computations. Additionally, added new statistic classes implementing the best of 10 repetitions approach. The statistics include Kolmogorov Smirn
The value for `meekPreventCycles` has been changed from false to true in both `Fges.java` and `FgesMb.java` to prevent cycles in Meek rules. Additionally, extensive comments and documentation have been introduced in several classes including `Comparison.java`, `GraphTransforms.java`, and `MeekRules.java` to clarify methods' purposes and parameters. Some unnecessary cycle checking code in `MeekRules.java` has been commented out.
An additional argument has been included in the compareFromSimulations method in the Comparison.java file. This change allows for a null value to be passed for improved flexibility and compatibility in certain situations where the previous parameters may not be required or available.
A new class, MagDgBicScore, has been added to handle the MAG Degenerate Gaussian BIC Score.
Several enhancements were made to the comments in the tetrad-gui package to provide better understanding of classes and methods. Method parameters in PathsAction and LinearAdjustmentRegressionEditor classes have also been restructured for enhanced functionality.
The main changes in this commit are the refactoring of graph comparison methods to make them more efficient and the adjustments to the collider orientation process. The graph comparison methods' refactoring includes the update of the `getComparisonGraph` method both in `Misclassifications.java` and `EdgewiseComparisonModel.java`. Additionally, the collider orientation process in the `LvLite.java` class has been updated to accommodate changes to the score metrics. The equality threshold has also been adjusted.
The function orientCollidersAndRemoveEdges has been renamed to orientAndRemove to more accurately reflect its functionality. The change has been reflected wherever this method is called throughout the codebase. This provides a more intuitive understanding of the method's role in the lvLite and lvLiteDsepFriendly classes.
The LvLite algorithm has undergone changes including importing Math.exp instead of Math.abs. Renamed "best_score" variable as "bestScore" and applied this convention throughout the score calculations. The condition to compare "bestScore" and "newScore" has also been updated where the variable "bayesFactor" is used. The "doRequiredOrientations" method has also been slightly refactored.
The term "equalityThreshold" was renamed to "bayesFactorThreshold" to avoid confusion and ensure consistency. The change affects the LV-Lite procedure where the new term "bayesFactorThreshold" is now used to prevent score drops more than 2 * Bayes factor. This is because the BIC scores are calculated using the formula 2L - c k ln N. The codebase was updated across multiple files accordingly.
The documentation inaccurately described the Bayes factor threshold in the LV-Lite procedure. This update corrects this by clarifying that it should be the Log Bayes factor threshold instead.
Refactored all instances of bayesFactorThreshold in the LvLite and LvLiteDsepFriendly classes to allowableThreshold. This includes method names, variable names, and comments. Also updated related classes and methods to reflect this change. Bayes Factor computations have been removed as they are no longer necessary with the new threshold metric. This change improves readability and ensures consistency across classes and methods.
…ed classes

This commit implements changes to GraphEditor, BFci, LvLiteDsepFriendly, GraphUtils, DagEditor, SpFci, SemGraphEditor, FciOrient, Fci, and GraspFci classes. The primary change involved is the introduction of rules and methods to repair faulty PAGs (probability assessment graphs). Such changes aid in handling cases where the PAG may have inconsistencies or errors, allowing for more robust graph evaluations. In addition, adjustments have been made to keybindings for pathsAction in various editors, changing them from ALT+P to ALT+T.
jdramsey added 27 commits August 5, 2024 01:10
Deleted several unused methods related to path blocking and simplified the SepsetFinder class implementation. This reduces code complexity and improves maintainability without impacting functionality.
Replaced non-directed edge addition with bidirected edge addition in GraphUtils as per Zhang 2008. Implemented a new Dijkstra algorithm in Dijkstra.java to improve performance in FciOrient, avoiding potential hangs on large graphs by finding uncovered paths.
Implemented a new static method `nondirectedGraph` in `GraphUtils` to convert graphs to have nondirected edges. Added a new class `AllEdgesNondirectedWrapper` and made necessary updates to configuration files to support the new functionality in both production and development environments.
Implemented a new static method `nondirectedGraph` in `GraphUtils` to convert graphs to have nondirected edges. Added a new class `AllEdgesNondirectedWrapper` and made necessary updates to configuration files to support the new functionality in both production and development environments.
Refactor FciOrient class to utilize a precomputed Dijkstra graph.
Implement `traverseNondirected` method and improve handling
of graph traversal in Dijkstra. Add verbose flag to Fci class for
debugging and adjust unit tests to reflect changes.
Renamed the Dijkstra utility class to FciOrientDijkstra to reflect its specific application in FciOrient rules R5, R9, and R10. Updated all references and implemented a more specialized Dijkstra's algorithm tailored for FCI orientation-related tasks in tetrad-lib.
Renamed the Dijkstra utility class to FciOrientDijkstra to reflect its specific application in FciOrient rules R5, R9, and R10. Updated all references and implemented a more specialized Dijkstra's algorithm tailored for FCI orientation-related tasks in tetrad-lib.
Renamed the Dijkstra utility class to FciOrientDijkstra to reflect its specific application in FciOrient rules R5, R9, and R10. Updated all references and implemented a more specialized Dijkstra's algorithm tailored for FCI orientation-related tasks in tetrad-lib.
Deleted the FciOrientDijkstra class and replaced its usage with R5R9Dijkstra in relevant files. Additionally, renamed FciOrientDataExaminationStrategy and related classes to R4Strategy for better clarity and consistency.
Renamed `R4Strategy` to `R0R4Strategy` to reflect its responsibility for both R0 and R4 rules, and updated all corresponding references in the codebase. Updated documentation to clarify the usage and functionality related to R0 and R4 orientation rules.
Deleted an unnecessary newline to improve code readability and maintain coding standards. No functional changes were made; this is a purely cosmetic update.
Implemented logic to handle 'o--' and '--o' edge specifications in `GraphUtils`. Refactored FCI orientation rules including rules R6 and R7 for better clarity and efficiency. Updated test cases in `TestFci` to reflect changes in edge orientation logic.
Enhanced `GraphUtils` to handle new edge types and updated the `Counts` array size to accommodate these changes. Additionally, modified the method signature of `FciOrient.orient` to remove an unnecessary return statement, improving the method's clarity and correctness.
Simplified `DiscriminatingPath` logic by modifying key methods to use more concise parameters. Removed verbose comments and refactored validation to improve readability and maintainability. Updated related classes to align with the new discriminating path structure.
Relocate R5R9Dijkstra from the util package to search.utils for better module organization. Updated all relevant import statements in FciOrient and SvarFciOrient accordingly.
Removed redundant comments and streamlined logic for discriminating path orientation across multiple files. Updated method signatures to reflect simplified logic and added validation checks in a centralized manner.
…k_2024_6-13

# Conflicts:
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/DagToPag.java
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/DiscriminatingPath.java
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/FciOrient.java
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/R0R4Strategy.java
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/R0R4StrategyScoreBased.java
#	tetrad-lib/src/main/java/edu/cmu/tetrad/search/utils/R0R4StrategyTestBased.java
The import statement for R5R9Dijkstra was not being used in the code. This cleanup improves code readability and maintainability by removing unnecessary dependencies.
Deleted the SvarFciOrient.java file which contained legacy code for the SvarFCI algorithm. This cleanup removes outdated and possibly unused code components to streamline the codebase.
Refactor method name for clarity in DiscriminatingPath class and update references in related code. This improves readability and better describes the method's purpose.
Updated the `DiscriminatingPath` class to include additional documentation, rename methods for clarity, and improve path checking logic. Enhanced comments and checks in relevant methods in `FciOrient`, `DagToPag`, and `R0R4StrategyTestBased` to ensure correctness and readability. Adjusted test cases in `TestFci` to align with new path orientation rules.
Updated edge orientation rules R1-R10 for better clarity and succinctness. Simplified comments and documentation, focusing on mathematical notation and removing verbose explanations. Improved implementation by removing redundant checks and consolidating functions where appropriate.
Changed the visibility of fciOrientbk method from private to public. This modification allows external classes to access and utilize the method for BK orientation in a graph.
Refactor `defVisible` logic, add detailed comments, and introduce helper methods to improve readability and maintainability. Also, include documentation for endpoint strategy in FciOrient.java.
Wrapped all logging statements with a verbosity check (`if (verbose)`) to ensure logs are generated only when the `verbose` flag is enabled. This reduces unnecessary logging and improves performance in non-verbose mode. The change affects multiple sections in the `LvLite.java` file, enhancing overall log management.
Added a newline for better code readability before a comment. No functional changes made to the existing logic.
Added Javadoc comments to several classes and methods across multiple files, including DefaultSetEndpointStrategy, SetEndpointStrategy, R5R9Dijkstra, GraphSearchUtils, SvarSetEndpointStrategy, R0R4Strategy, FciOrientDijkstra, Paths, and Edges. This improves code documentation and provides useful information about the purpose and usage of classes and their methods.
@jdramsey jdramsey merged commit 66f42ea into development Aug 11, 2024
@jdramsey jdramsey self-assigned this Aug 11, 2024
@jdramsey jdramsey deleted the joe_work_2024_6-13 branch August 11, 2024 06:00
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.

1 participant