Add Claude local discretization analysis

Fix smell issues in markdown and python

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - *mdlp_min_length*: minimum length of a partition in MDLP algorithm to be evaluated for partition.
   - *mdlp_max_depth*: maximum level of recursion in MDLP algorithm.
 - Remove vcpkg as a dependency manager, now the library is built with Conan package manager and CMake.
+- Add `build_type` option to the sample target in the Makefile to allow building in *Debug* or *Release* mode. Default is *Debug*.
 
 ## [1.1.1] - 2025-05-20
 

CMakeLists.txt

@@ -18,16 +18,18 @@ set(CMAKE_CXX_EXTENSIONS                  OFF)
 set(CMAKE_EXPORT_COMPILE_COMMANDS          ON)
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TORCH_CXX_FLAGS}")
 SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
-set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -Ofast")
+set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")
-if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-default-inline")
+
-endif()
+if (CMAKE_BUILD_TYPE STREQUAL "Debug")
+    MESSAGE("Debug mode")
+else(CMAKE_BUILD_TYPE STREQUAL "Debug")
+    MESSAGE("Release mode")
+endif (CMAKE_BUILD_TYPE STREQUAL "Debug")
 
 # Options
 # -------
 option(ENABLE_TESTING "Unit testing build"                       OFF)
-option(CODE_COVERAGE "Collect coverage from test library"        OFF)
-
     
 find_package(Torch CONFIG REQUIRED)
 if(NOT TARGET torch::torch)
@@ -63,23 +65,21 @@ target_link_libraries(bayesnet
     arff-files::arff-files
 )
 
+
+
 # Testing
 # -------
-if (CMAKE_BUILD_TYPE STREQUAL "Debug")
-    MESSAGE("Debug mode")
-else(CMAKE_BUILD_TYPE STREQUAL "Debug")
-    MESSAGE("Release mode")
-endif (CMAKE_BUILD_TYPE STREQUAL "Debug")
 if (ENABLE_TESTING)
     MESSAGE(STATUS "Testing enabled")
     set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fprofile-arcs -ftest-coverage -fno-elide-constructors")
+    if (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+        set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-default-inline")
+    endif()
     find_package(Catch2 CONFIG REQUIRED)
     find_package(arff-files CONFIG REQUIRED)
     enable_testing()
     include(CTest)
     add_subdirectory(tests)
-else(ENABLE_TESTING)
-    
 endif (ENABLE_TESTING)
 
 # Installation

Makefile

@@ -1,6 +1,6 @@
 SHELL := /bin/bash
 .DEFAULT_GOAL := help
-.PHONY: viewcoverage coverage setup help install uninstall diagrams buildr buildd test clean updatebadge doc doc-install init clean-test conan-debug conan-release conan-create conan-upload conan-clean conan-sample
+.PHONY: viewcoverage coverage setup help install uninstall diagrams buildr buildd test clean updatebadge doc doc-install init clean-test debug release conan-create conan-upload conan-clean sample
 
 f_release = build_Release
 f_debug = build_Debug
@@ -31,6 +31,14 @@ define ClearTests
 	fi ; 
 endef
 
+define setup_target
+	@echo ">>> Setup the project for $(1)..."
+	@if [ -d $(2) ]; then rm -fr $(2); fi
+	@conan install . --build=missing -of $(2) -s build_type=$(1)
+	@cmake -S . -B $(2) -DCMAKE_TOOLCHAIN_FILE=$(2)/build/$(1)/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=$(1) -D$(3)
+	@echo ">>> Done"
+endef
+
 setup: ## Install dependencies for tests and coverage
 	@if [ "$(shell uname)" = "Darwin" ]; then \
 		brew install gcovr; \
@@ -57,6 +65,12 @@ clean: ## Clean the project
 # Build targets
 # =============
 
+debug: ## Setup debug version using Conan
+	@$(call setup_target,"Debug","$(f_debug)","ENABLE_TESTING=ON")
+
+release: ## Setup release version using Conan
+	@$(call setup_target,"Release","$(f_release)","ENABLE_TESTING=OFF")
+
 buildd: ## Build the debug targets
 	cmake --build $(f_debug) --config Debug -t $(app_targets) --parallel $(CMAKE_BUILD_PARALLEL_LEVEL)
 
@@ -160,6 +174,7 @@ doc: ## Generate documentation
 	@echo ">>> Done";
 
 diagrams: ## Create an UML class diagram & dependency of the project (diagrams/BayesNet.png)
+	@echo ">>> Creating diagrams..."
 	@which $(plantuml) || (echo ">>> Please install plantuml"; exit 1)
 	@which $(dot) || (echo ">>> Please install graphviz"; exit 1)
 	@which $(clang-uml) || (echo ">>> Please install clang-uml"; exit 1)
@@ -172,6 +187,7 @@ diagrams: ## Create an UML class diagram & dependency of the project (diagrams/B
 	$(MAKE) debug
 	cd $(f_debug) && cmake .. --graphviz=dependency.dot 
 	@$(dot) -Tsvg $(f_debug)/dependency.dot.BayesNet -o $(f_diagrams)/dependency.svg
+	@echo ">>> Done";
 
 docdir = ""
 doc-install: ## Install documentation
@@ -190,61 +206,28 @@ doc-install: ## Install documentation
 # Conan package manager targets
 # =============================
 
-debug:             ## Build debug version using Conan
-	@echo ">>> Building *Debug* BayesNet with Conan..."
-	@rm -rf $(f_debug)            # wipe previous tree
-	@conan install . \
-	            -s build_type=Debug \
-	            --build=missing \
-	            -of $(f_debug) \
-				--profile=debug
-	@cmake -S . -B $(f_debug) \
-	       -DCMAKE_BUILD_TYPE=Debug \
-	       -DENABLE_TESTING=ON \
-	       -DCODE_COVERAGE=ON \
-	       -DCMAKE_TOOLCHAIN_FILE=$(f_debug)/build/Debug/generators/conan_toolchain.cmake
-	@echo ">>> Done"
-
-release: ## Build release version using Conan
-	@echo ">>> Building Release BayesNet with Conan..."
-	@conan install . \
-	            -s build_type=Release \
-	            --build=missing \
-	            -of $(f_debug) \
-				--profile=release
-	@if [ -d ./$(f_release) ]; then rm -rf ./$(f_release); fi
-	@mkdir $(f_release)
-	@conan install . -s build_type=Release --build=missing -of $(f_release)
-	@cmake -S . -B $(f_release) -D CMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=$(f_release)/build/Release/generators/conan_toolchain.cmake
-	@echo ">>> Done"
-
 conan-create: ## Create Conan package
 	@echo ">>> Creating Conan package..."
-	@conan create . --build=missing -tf "" --profile=release
+	@conan create . --build=missing -tf "" -s:a build_type=Release
-	@conan create . --build=missing -tf "" --profile=debug -o "&:enable_coverage=False" -o "&:enable_testing=False"
+	@conan create . --build=missing -tf "" -s:a build_type=Debug -o "&:enable_coverage=False" -o "&:enable_testing=False"
-	@echo ">>> Done"
-
-profile ?= release
-remote ?= Cimmeria
-conan-upload: ## Upload package to Conan remote (profile=release remote=Cimmeria)
-	@echo ">>> Uploading to Conan remote $(remote) with profile $(profile)..."
-	@conan upload bayesnet/$(grep version conanfile.py | cut -d'"' -f2) -r $(remote) --confirm
 	@echo ">>> Done"
 
 conan-clean: ## Clean Conan cache and build folders
 	@echo ">>> Cleaning Conan cache and build folders..."
 	@conan remove "*" --confirm
+	@conan cache clean
 	@if test -d "$(f_release)" ; then rm -rf "$(f_release)"; fi
 	@if test -d "$(f_debug)" ; then rm -rf "$(f_debug)"; fi
 	@echo ">>> Done"
 
 fname = "tests/data/iris.arff"
 model = "TANLd"
+build_type = "Debug"
 sample: ## Build sample with Conan
 	@echo ">>> Building Sample with Conan...";
 	@if [ -d ./sample/build ]; then rm -rf ./sample/build; fi
-	@cd sample && conan install . --output-folder=build --build=missing
+	@cd sample && conan install . --output-folder=build --build=missing -s build_type=$(build_type) -o "&:enable_coverage=False" -o "&:enable_testing=False"
-	@cd sample && cmake -B build -S . -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=build/conan_toolchain.cmake && \
+	@cd sample && cmake -B build -S . -DCMAKE_BUILD_TYPE=$(build_type) -DCMAKE_TOOLCHAIN_FILE=build/conan_toolchain.cmake && \
 	cmake --build build -t bayesnet_sample
 	sample/build/bayesnet_sample $(fname) $(model)
 	@echo ">>> Done";
@@ -267,4 +250,4 @@ help: ## Show help message
 		printf "%-20s %s" $$help_command ; \
 		printf '\033[0m'; \
 		printf "%s\n" $$help_info; \
-	done
+	done

local_discretization_analysis.md

@@ -0,0 +1,235 @@
+# Local Discretization Analysis - BayesNet Library
+
+## Overview
+
+This document analyzes the local discretization implementation in the BayesNet library, specifically focusing on the `Proposal.cc` implementation, and evaluates the feasibility of implementing an iterative discretization approach.
+
+## Current Local Discretization Implementation
+
+### Core Architecture
+
+The local discretization functionality is implemented through a **Proposal class** (`bayesnet/classifiers/Proposal.h`) that serves as a mixin/base class for creating "Ld" (Local Discretization) variants of existing classifiers.
+
+### Key Components
+
+#### 1. The Proposal Class
+- **Purpose**: Handles continuous data by applying local discretization using discretization algorithms
+- **Dependencies**: Uses the `fimdlp` library for discretization algorithms
+- **Supported Algorithms**:
+  - **MDLP** (Minimum Description Length Principle) - Default
+  - **BINQ** - Quantile-based binning
+  - **BINU** - Uniform binning
+
+#### 2. Local Discretization Variants
+
+The codebase implements Ld variants using multiple inheritance:
+
+**Individual Classifiers:**
+- `TANLd` - Tree Augmented Naive Bayes with Local Discretization
+- `KDBLd` - K-Dependence Bayesian with Local Discretization  
+- `SPODELd` - Super-Parent One-Dependence Estimator with Local Discretization
+
+**Ensemble Classifiers:**
+- `AODELd` - Averaged One-Dependence Estimator with Local Discretization
+
+### Implementation Pattern
+
+All Ld variants follow a consistent pattern using **multiple inheritance**:
+
+```cpp
+class TANLd : public TAN, public Proposal {
+    // Inherits from both the base classifier and Proposal
+};
+```
+
+### Two-Phase Discretization Process
+
+#### Phase 1: Initial Discretization (`fit_local_discretization`)
+- Each continuous feature is discretized independently using the chosen algorithm
+- Creates initial discrete dataset
+- Uses only class labels for discretization decisions
+
+#### Phase 2: Network-Aware Refinement (`localDiscretizationProposal`)
+- After building the initial Bayesian network structure
+- Features are re-discretized considering their parent nodes in the network
+- Uses topological ordering to ensure proper dependency handling
+- Creates more informed discretization boundaries based on network relationships
+
+### Hyperparameter Support
+
+The Proposal class supports several configurable hyperparameters:
+- `ld_algorithm`: Choice of discretization algorithm (MDLP, BINQ, BINU)
+- `ld_proposed_cuts`: Number of proposed cuts for discretization
+- `mdlp_min_length`: Minimum interval length for MDLP
+- `mdlp_max_depth`: Maximum depth for MDLP tree
+
+## Current Implementation Strengths
+
+1. **Sophisticated Approach**: Considers network structure in discretization decisions
+2. **Modular Design**: Clean separation through Proposal class mixin
+3. **Multiple Algorithm Support**: Flexible discretization strategies
+4. **Proper Dependency Handling**: Topological ordering ensures correct processing
+5. **Well-Integrated**: Seamless integration with existing classifier architecture
+
+## Areas for Improvement
+
+### Code Quality Issues
+
+1. **Dead Code**: Line 161 in `Proposal.cc` contains unused variable `allDigits`
+2. **Performance Issues**: 
+   - String concatenation in tight loop (lines 82-84) using `+=` operator
+   - Memory allocations could be optimized
+   - Tensor operations could be batched better
+3. **Error Handling**: Could be more robust with better exception handling
+
+### Algorithm Clarity
+
+1. **Logic Clarity**: The `upgrade` flag logic could be more descriptive
+2. **Variable Naming**: Some variables need more descriptive names
+3. **Documentation**: Better inline documentation of the two-phase process
+4. **Method Complexity**: `localDiscretizationProposal` method is quite long and complex
+
+### Suggested Code Improvements
+
+```cpp
+// Instead of string concatenation in loop:
+for (auto idx : indices) {
+    for (int i = 0; i < Xf.size(1); ++i) {
+        yJoinParents[i] += to_string(pDataset.index({ idx, i }).item<int>());
+    }
+}
+
+// Consider using stringstream or pre-allocation:
+std::stringstream ss;
+for (auto idx : indices) {
+    for (int i = 0; i < Xf.size(1); ++i) {
+        ss << pDataset.index({ idx, i }).item<int>();
+        yJoinParents[i] = ss.str();
+        ss.str("");
+    }
+}
+```
+
+## Iterative Discretization Proposal
+
+### Concept
+
+Implement an iterative process: discretize → build model → re-discretize → rebuild model → repeat until convergence.
+
+### Feasibility Assessment
+
+**Highly Feasible** - The current implementation already provides a solid foundation with its two-phase approach, making extension straightforward.
+
+### Proposed Implementation Strategy
+
+```cpp
+class IterativeProposal : public Proposal {
+public:
+    struct ConvergenceParams {
+        int max_iterations = 10;
+        double tolerance = 1e-6;
+        bool check_network_structure = true;
+        bool check_discretization_stability = true;
+    };
+
+private:
+    map<string, vector<int>> iterativeLocalDiscretization(const torch::Tensor& y) {
+        auto states = fit_local_discretization(y);  // Initial discretization
+        Network previousModel, currentModel;
+        int iteration = 0;
+        
+        do {
+            previousModel = currentModel;
+            
+            // Build model with current discretization
+            const torch::Tensor weights = torch::full({ pDataset.size(1) }, 1.0 / pDataset.size(1), torch::kDouble);
+            currentModel.fit(pDataset, weights, pFeatures, pClassName, states, Smoothing_t::ORIGINAL);
+            
+            // Apply local discretization based on current model
+            auto newStates = localDiscretizationProposal(states, currentModel);
+            
+            // Check for convergence
+            if (hasConverged(previousModel, currentModel, states, newStates)) {
+                break;
+            }
+            
+            states = newStates;
+            iteration++;
+            
+        } while (iteration < convergenceParams.max_iterations);
+        
+        return states;
+    }
+    
+    bool hasConverged(const Network& prev, const Network& curr, 
+                     const map<string, vector<int>>& oldStates,
+                     const map<string, vector<int>>& newStates) {
+        // Implementation of convergence criteria
+        return checkNetworkStructureConvergence(prev, curr) && 
+               checkDiscretizationStability(oldStates, newStates);
+    }
+};
+```
+
+### Convergence Criteria Options
+
+1. **Network Structure Comparison**: Compare edge sets between iterations
+   ```cpp
+   bool checkNetworkStructureConvergence(const Network& prev, const Network& curr) {
+       // Compare adjacency matrices or edge lists
+       return prev.getEdges() == curr.getEdges();
+   }
+   ```
+
+2. **Discretization Stability**: Check if cut points change significantly
+   ```cpp
+   bool checkDiscretizationStability(const map<string, vector<int>>& oldStates,
+                                    const map<string, vector<int>>& newStates) {
+       for (const auto& [feature, states] : oldStates) {
+           if (states != newStates.at(feature)) {
+               return false;
+           }
+       }
+       return true;
+   }
+   ```
+
+3. **Performance Metrics**: Monitor accuracy/likelihood convergence
+4. **Maximum Iterations**: Prevent infinite loops
+
+### Expected Benefits
+
+1. **Better Discretization Quality**: Each iteration refines boundaries based on learned dependencies
+2. **Improved Model Accuracy**: More informed discretization leads to better classification
+3. **Adaptive Process**: Automatically finds optimal discretization-model combination
+4. **Principled Approach**: Theoretically sound iterative refinement
+5. **Reduced Manual Tuning**: Less need for hyperparameter optimization
+
+### Implementation Considerations
+
+1. **Convergence Detection**: Need robust criteria to detect when to stop
+2. **Performance Impact**: Multiple iterations increase computational cost
+3. **Overfitting Prevention**: May need regularization to avoid over-discretization
+4. **Stability Guarantees**: Ensure the process doesn't oscillate indefinitely
+5. **Memory Management**: Handle multiple model instances efficiently
+
+### Integration Strategy
+
+1. **Backward Compatibility**: Keep existing two-phase approach as default
+2. **Optional Feature**: Add iterative mode as configurable option
+3. **Hyperparameter Extension**: Add convergence-related parameters
+4. **Testing Framework**: Comprehensive testing on standard datasets
+
+## Conclusion
+
+The current local discretization implementation in BayesNet is well-designed and functional, providing a solid foundation for the proposed iterative enhancement. The iterative approach would significantly improve the quality of discretization by creating a feedback loop between model structure and discretization decisions.
+
+The implementation is highly feasible given the existing architecture, and the expected benefits justify the additional computational complexity. The key to success will be implementing robust convergence criteria and maintaining the modularity of the current design.
+
+## Recommendations
+
+1. **Immediate Improvements**: Fix code quality issues and optimize performance bottlenecks
+2. **Iterative Implementation**: Develop the iterative approach as an optional enhancement
+3. **Comprehensive Testing**: Validate improvements on standard benchmark datasets
+4. **Documentation**: Enhance inline documentation and user guides
+5. **Performance Profiling**: Monitor computational overhead and optimize where necessary