Skip to content
Merged
106 changes: 59 additions & 47 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,68 +252,80 @@ Tests added: `tests/core/test_grid.c` with 16 unit tests.
- `examples/*.c` - Example error handling
- `README.md` - API documentation updates

### 0.6 Structured Logging & Diagnostics (P2)
### 0.6 Structured Logging & Diagnostics (P2)

**Status:** Partial - `cfd_set_log_callback()` API exists but not used consistently
**Status:** Complete

**Current Issues:**
**Implementation Tasks:**

- Raw `fprintf(stderr, ...)` and `snprintf()` scattered throughout codebase
- No log levels (can't filter INFO vs WARNING vs ERROR)
- No redirection (always stderr, can't send to file/syslog/GUI)
- No timestamps or structured metadata
- Not thread-safe (garbled output from multiple threads)
- Mixed purposes (diagnostics vs error messages)
- [x] Define `cfd_log()` API with log levels and component tags
- [x] Implement default console handler (stderr for WARNING/ERROR, stdout for DEBUG/INFO)
- [x] Add thread-safe logging (atomic global log level, per-thread callbacks)
- [x] Replace all `fprintf(stderr, ...)` calls with `cfd_log()`
- [x] Replace diagnostic `printf()` calls with `cfd_log()`
- [x] Add log filtering by level (suppress DEBUG in production)
- [x] Support custom log handlers via `cfd_set_log_callback_ex()` (with component)
- [ ] Add log filtering by component (e.g., only show "boundary" logs) — deferred
- [ ] Add timestamps and colored output — deferred
- [ ] Add structured data API for metrics (convergence stats, timings) — deferred

**Proposed Structured Logging API:**
**API added:**

```c
// Log levels
// Log levels (renumbered to include DEBUG)
typedef enum {
CFD_LOG_DEBUG = 0,
CFD_LOG_INFO = 1,
CFD_LOG_WARNING = 2,
CFD_LOG_ERROR = 3
CFD_LOG_LEVEL_DEBUG = 0,
CFD_LOG_LEVEL_INFO = 1,
CFD_LOG_LEVEL_WARNING = 2,
CFD_LOG_LEVEL_ERROR = 3
} cfd_log_level_t;

// Logging function with structured metadata
// Core logging function with printf-style formatting
void cfd_log(cfd_log_level_t level, const char* component,
const char* format, ...);

// Example usage
cfd_log(CFD_LOG_WARNING, "poisson_solver",
"Failed to converge (grid %zux%zu, dt=%.4e)",
nx, ny, dt);
const char* fmt, ...);

// Global log level control (atomic, thread-safe)
void cfd_set_log_level(cfd_log_level_t level);
cfd_log_level_t cfd_get_log_level(void);

// Extended callback with component tag
typedef void (*cfd_log_callback_ex_t)(cfd_log_level_t level,
const char* component, const char* message);
void cfd_set_log_callback_ex(cfd_log_callback_ex_t callback);

// Convenience macros
CFD_LOG_DEBUG(component, ...)
CFD_LOG_INFO(component, ...)
CFD_LOG_WARNING(component, ...)
CFD_LOG_ERROR(component, ...)
```

**Implementation Tasks:**

- [ ] Define `cfd_log()` API with log levels and component tags
- [ ] Implement default console handler (with timestamps, colored output)
- [ ] Add thread-safe logging (mutex-protected or per-thread buffers)
- [ ] Replace all `fprintf(stderr, ...)` calls with `cfd_log()`
- [ ] Replace diagnostic `snprintf()` with `cfd_log()` where appropriate
- [ ] Add log filtering by level (suppress DEBUG in production)
- [ ] Add log filtering by component (e.g., only show "boundary" logs)
- [ ] Support custom log handlers via callback
- [ ] Add structured data API for metrics (convergence stats, timings)
**Design decisions:**

**Benefits:**

- Users can redirect logs to files, syslog, or application UI
- Fine-grained control (enable DEBUG for specific components)
- Better debugging (timestamps, thread IDs, component context)
- Thread-safe by design
- Statistics aggregation ("Poisson failed 15 times this run")
- Can mute logs entirely for embedded/production use
- Default log level is INFO (DEBUG suppressed unless explicitly enabled)
- Per-thread legacy callback takes priority over global extended callback
- `cfd_error()` always sets thread-local error state regardless of log level filter
- Convergence verbose output uses `CFD_LOG_DEBUG` with `if (params->verbose)` guard
- Component tags: "gpu", "boundary", "poisson", "projection", "solver", "simd"
- Default output format: `LEVEL [component]: message`

**Files to Modify:**
**Files modified:**

- `lib/src/solvers/linear/cpu/linear_solver_*.c` - Replace fprintf
- `lib/src/solvers/navier_stokes/cpu/solver_*.c` - Replace fprintf
- `lib/src/solvers/navier_stokes/omp/solver_*.c` - Replace fprintf
- `lib/src/solvers/navier_stokes/avx2/solver_*.c` - Replace fprintf
- `tests/validation/lid_driven_cavity_common.h` - Replace snprintf for diagnostics
- `lib/include/cfd/core/logging.h` — Extended API with DEBUG level, `cfd_log()`, macros
- `lib/src/core/logging.c` — `cfd_log()` implementation, atomic log level, extended callback
- `lib/src/api/solver_registry.c` — 5 fprintf → CFD_LOG_ERROR/WARNING
- `lib/src/boundary/boundary_conditions.c` — 1 fprintf → CFD_LOG_ERROR
- `lib/src/solvers/linear/linear_solver.c` — 2 fprintf/printf → CFD_LOG_ERROR/DEBUG
- `lib/src/solvers/linear/simd/linear_solver_simd_dispatch.c` — 1 fprintf → CFD_LOG_DEBUG
- `lib/src/solvers/linear/cpu/linear_solver_cg.c` — 1 printf → CFD_LOG_DEBUG
- `lib/src/solvers/linear/cpu/linear_solver_bicgstab.c` — 1 printf → CFD_LOG_DEBUG
- `lib/src/solvers/linear/avx2/linear_solver_cg_avx2.c` — 1 printf → CFD_LOG_DEBUG
- `lib/src/solvers/linear/neon/linear_solver_cg_neon.c` — 1 printf → CFD_LOG_DEBUG
- `lib/src/solvers/linear/omp/linear_solver_cg_omp.c` — 1 printf → CFD_LOG_DEBUG
- `lib/src/solvers/navier_stokes/avx2/solver_explicit_euler_avx2.c` — 4 printf → CFD_LOG_INFO
- `lib/src/solvers/navier_stokes/avx2/solver_projection_avx2.c` — 1 fprintf → CFD_LOG_WARNING
- `lib/src/solvers/navier_stokes/avx2/solver_rk2_avx2.c` — 2 printf → CFD_LOG_INFO
- `tests/core/test_logging.c` — 11 new tests (16 total)

---

Expand Down
81 changes: 73 additions & 8 deletions lib/include/cfd/core/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,92 @@

#include "cfd/cfd_export.h"

/* Printf format checking attribute */
#if defined(__GNUC__) || defined(__clang__)
#define CFD_PRINTF_FORMAT(fmt_idx, arg_idx) __attribute__((format(printf, fmt_idx, arg_idx)))
#else
#define CFD_PRINTF_FORMAT(fmt_idx, arg_idx)
#endif

#ifdef __cplusplus
extern "C" {
#endif

// Log levels
typedef enum { CFD_LOG_LEVEL_INFO, CFD_LOG_LEVEL_WARNING, CFD_LOG_LEVEL_ERROR } cfd_log_level_t;
/* ============================================================================
* LOG LEVELS
* ============================================================================ */

typedef enum {
CFD_LOG_LEVEL_DEBUG = 0,
CFD_LOG_LEVEL_INFO = 1,
CFD_LOG_LEVEL_WARNING = 2,
CFD_LOG_LEVEL_ERROR = 3
} cfd_log_level_t;

/* ============================================================================
* CALLBACK TYPES
* ============================================================================ */

// Log callback function type
/** Per-thread callback (legacy — no component info) */
typedef void (*cfd_log_callback_t)(cfd_log_level_t level, const char* message);

// Set a custom logging callback for the CURRENT THREAD (pass NULL to reset to default stderr
// output)
/** Extended callback with component tag */
typedef void (*cfd_log_callback_ex_t)(cfd_log_level_t level, const char* component,
const char* message);

/* ============================================================================
* CORE LOGGING API
* ============================================================================ */

/**
* Log a message with printf-style formatting, a log level, and a component tag.
*
* Messages below the global log level are suppressed (fast atomic check).
*
* Dispatch priority:
* 1. Per-thread callback (set via cfd_set_log_callback)
* 2. Global extended callback (set via cfd_set_log_callback_ex)
* 3. Default handler (stderr for WARNING/ERROR, stdout for DEBUG/INFO)
*
* @param level Log level (DEBUG, INFO, WARNING, ERROR)
* @param component Component tag (e.g., "poisson", "boundary") or NULL
* @param fmt printf-style format string
*/
CFD_LIBRARY_EXPORT void cfd_log(cfd_log_level_t level, const char* component, const char* fmt,
...) CFD_PRINTF_FORMAT(3, 4);

/** Set the global minimum log level (default: CFD_LOG_LEVEL_INFO). Thread-safe. */
CFD_LIBRARY_EXPORT void cfd_set_log_level(cfd_log_level_t level);

/** Get the current global minimum log level. Thread-safe. */
CFD_LIBRARY_EXPORT cfd_log_level_t cfd_get_log_level(void);

/** Set a global extended callback (receives component info). Pass NULL to reset. */
CFD_LIBRARY_EXPORT void cfd_set_log_callback_ex(cfd_log_callback_ex_t callback);

/* ============================================================================
* CONVENIENCE MACROS
* ============================================================================ */

#define CFD_LOG_DEBUG(component, ...) cfd_log(CFD_LOG_LEVEL_DEBUG, (component), __VA_ARGS__)
#define CFD_LOG_INFO(component, ...) cfd_log(CFD_LOG_LEVEL_INFO, (component), __VA_ARGS__)
#define CFD_LOG_WARNING(component, ...) cfd_log(CFD_LOG_LEVEL_WARNING, (component), __VA_ARGS__)
#define CFD_LOG_ERROR(component, ...) cfd_log(CFD_LOG_LEVEL_ERROR, (component), __VA_ARGS__)

/* ============================================================================
* LEGACY API (backward-compatible convenience wrappers)
* ============================================================================ */

/** Set a per-thread logging callback (pass NULL to reset to default stderr output) */
CFD_LIBRARY_EXPORT void cfd_set_log_callback(cfd_log_callback_t callback);

// Log error message and set thread-local error state (does NOT exit)
/** Log error message and set thread-local error state (does NOT exit) */
CFD_LIBRARY_EXPORT void cfd_error(const char* message);

// Print warning message
/** Print warning message */
CFD_LIBRARY_EXPORT void cfd_warning(const char* message);

// Print info message
/** Print info message */
CFD_LIBRARY_EXPORT void cfd_info(const char* message);

#ifdef __cplusplus
Expand Down
11 changes: 6 additions & 5 deletions lib/src/api/solver_registry.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "cfd/core/filesystem.h"
#include "cfd/core/gpu_device.h"
#include "cfd/core/grid.h"
#include "cfd/core/logging.h"
#include "cfd/core/memory.h"
#include "cfd/solvers/navier_stokes_solver.h"
#include "cfd/solvers/poisson_solver.h"
Expand Down Expand Up @@ -946,7 +947,7 @@ static cfd_status_t gpu_euler_init(ns_solver_t* solver, const grid* grid, const
ctx->gpu_ctx = gpu_solver_create(grid->nx, grid->ny, grid->nz, &ctx->gpu_config_t);
if (!ctx->gpu_ctx) {
cfd_free(ctx);
fprintf(stderr, "GPU Euler init: Failed to create GPU context\n");
CFD_LOG_ERROR("gpu", "GPU Euler init: Failed to create GPU context");
return CFD_ERROR_UNSUPPORTED;
}
}
Expand Down Expand Up @@ -1006,13 +1007,13 @@ static cfd_status_t gpu_euler_step(ns_solver_t* solver, flow_field* field, const
}
}
// GPU operation failed
fprintf(stderr, "GPU Euler step: GPU operation failed\n");
CFD_LOG_ERROR("gpu", "GPU Euler step: GPU operation failed");
return CFD_ERROR_INVALID;
}

// GPU not available - could be: CUDA not compiled in, gpu_should_use() returned false,
// or GPU initialization failed
fprintf(stderr, "GPU Euler step: GPU solver not initialized\n");
CFD_LOG_ERROR("gpu", "GPU Euler step: GPU solver not initialized");
return CFD_ERROR_UNSUPPORTED;
}

Expand Down Expand Up @@ -1051,7 +1052,7 @@ static cfd_status_t gpu_euler_solve(ns_solver_t* solver, flow_field* field, cons

// GPU not available - could be: CUDA not compiled in, gpu_should_use() returned false,
// or GPU initialization failed
fprintf(stderr, "GPU Euler solve: GPU solver not initialized\n");
CFD_LOG_ERROR("gpu", "GPU Euler solve: GPU solver not initialized");
return CFD_ERROR_UNSUPPORTED;
}

Expand Down Expand Up @@ -1335,7 +1336,7 @@ static cfd_status_t projection_omp_init(ns_solver_t* solver, const grid* grid,
poisson_solver_t* test_solver = poisson_solver_create(
POISSON_METHOD_CG, POISSON_BACKEND_OMP);
if (!test_solver) {
fprintf(stderr, "projection_omp_init: OMP CG Poisson solver not available\n");
CFD_LOG_WARNING("projection", "OMP CG Poisson solver not available");
return CFD_ERROR_UNSUPPORTED;
}
poisson_solver_destroy(test_solver);
Expand Down
2 changes: 1 addition & 1 deletion lib/src/boundary/boundary_conditions.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static void default_error_handler(bc_error_code_t error_code,
const char* message,
void* user_data) {
(void)user_data; /* Unused in default handler */
fprintf(stderr, "BC ERROR [%d] in %s: %s\n", (int)error_code, function, message);
CFD_LOG_ERROR("boundary", "BC [%d] in %s: %s", (int)error_code, function, message);
}

void bc_set_error_handler(bc_error_handler_t handler, void* user_data) {
Expand Down
25 changes: 25 additions & 0 deletions lib/src/core/cfd_threading_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ static inline void cfd_atomic_store(cfd_atomic_int* ptr, int value) {
InterlockedExchange(ptr, value);
}

/* Atomic pointer operations (for function pointer globals) */
typedef void* volatile cfd_atomic_ptr;

static inline void* cfd_atomic_ptr_load(const cfd_atomic_ptr* ptr) {
return InterlockedCompareExchangePointer((volatile PVOID*)ptr, NULL, NULL);
}

static inline void cfd_atomic_ptr_store(cfd_atomic_ptr* ptr, void* value) {
InterlockedExchangePointer(ptr, value);
}
Comment thread
shaia marked this conversation as resolved.

#else
#include <stdatomic.h>

Expand All @@ -44,6 +55,20 @@ static inline void cfd_atomic_store(cfd_atomic_int* ptr, int value) {
atomic_store(ptr, value);
}

/* Atomic pointer operations (for function pointer globals).
* Uses uintptr_t to avoid _Atomic(void*) which can cause TSan issues. */
#include <stdint.h>

typedef _Atomic(uintptr_t) cfd_atomic_ptr;

static inline void* cfd_atomic_ptr_load(cfd_atomic_ptr* ptr) {
return (void*)atomic_load(ptr);
}
Comment thread
shaia marked this conversation as resolved.

static inline void cfd_atomic_ptr_store(cfd_atomic_ptr* ptr, void* value) {
atomic_store(ptr, (uintptr_t)value);
}

#endif

#endif // CFD_THREADING_INTERNAL_H
Loading
Loading