Commit 071bf07e authored by Gaetano Scandariato's avatar Gaetano Scandariato
Browse files

bug fixes

parent c3e520ba
Loading
Loading
Loading
Loading
Loading

.kilo/cluster_plotting_plan.md

deleted100644 → 0
+0 −65
Changes for .kilo/cluster_plotting_plan.md: 0 added lines, 65 removed lines.
Original line number Diff line number Diff line
# Plan: Autodetect Data Clusters for Time-Series Plotting

## Objective
Modify the plotting logic in `plot_models.py` to automatically detect data clusters and create a "zoomed-in" view of these clusters. This applies **only** for per-instrument plots. This will eliminate uninformative gaps in the time-series plots while maintaining the overall figure structure and the 6:1 ratio between the left (time-series) and right (diagnostics/KDE) columns.

## Requirements
1. **Cluster Detection**: Use `detect_data_clusters` from `model_math.py` to identify contiguous blocks of data.
2. **Layout Preservation**: 
   - Keep the 6:1 ratio between the left and right columns.
   - The right column (KDE/diagnostics) remains a single panel per row.
   - Each row of the left column must be split into subpanels, one per detected cluster.
3. **Proportional Widths**: The width of each subpanel must be proportional to the timespan of the corresponding data cluster.
4. **Zoom-in Effect**: 
   - Iterate the current plotting procedure over these subpanels.
   - Set `xlim` for each subpanel to the timespan of its corresponding cluster, adding a buffer of 5% of the cluster's timespan to the left and right.
5. **Shared Y-Axis**: Subpanels within the same row must share the same y-axis.
6. **No Logic Modification**: The actual plotting commands (errorbars, lines, fills) must remain unchanged; only the axes they are plotted on and their limits are modified.

## Implementation Plan

At the end of each step, run `/dati/pyaetnaEnv/bin/python /dati/software/pyLib/PyAETNA/runPyAETNA.py` and do not proceed until all bugs are fixed.

### 1. Integration of Cluster Detection
#### Status: COMPLETE
- Import `detect_data_clusters` into `plot_models.py`.
- In `plot_models()`, after sorting the data by time (`t`), call `detect_data_clusters(t)` to get the list of cluster boundaries.
- Calculate the total timespan of all clusters to determine the proportional widths.
- *Note: While clusters are detected for any selection, the zoomed-in layout will be applied only when plotting per-instrument data (where gaps are most prevalent).*

### 2. Modification of Figure Layout
#### Status: COMPLETE
- The current `create_figure_layout` likely creates a grid of axes. This needs to be updated or wrapped.
- Instead of a single `axes[i]` for the left column, we will need a list of sub-axes for each row `i`.
- Use `matplotlib.gridspec.GridSpec` or `subplot_mosaic` to create a layout where:
    - The right column is a fixed width.
    - The left column is divided into $N$ sub-columns (where $N$ is the number of clusters), with widths proportional to the cluster durations.

### 3. Refactoring the Plotting Loop
#### Status: COMPLETE
- Wrap the existing plotting logic for each panel (Panel 0, Panel 1, Panel 2) in a loop over the detected clusters.
- This looping logic must be applied regardless of the number of clusters (including when only one cluster is detected) to ensure consistent code paths.
- For each cluster $C_j$:
    - Select the sub-axis $Ax_{i,j}$ corresponding to row $i$ and cluster $j$.
    - Execute the existing plotting code on $Ax_{i,j}$.
    - Set `Ax_{i,j}.set_xlim(C_j.start, C_j.end)`.
    - Ensure `sharey` is enabled across $Ax_{i,j}$ for all $j$ in row $i$.

### 4. Handling Specific Panels
#### Status: COMPLETE
- **Panel 0 (Raw Data)**: Apply the same cluster-splitting logic if it is visible.
- **Panel 1 (Detrended)**: Split into subpanels per cluster.
- **Panel 2 (Residuals)**: Split into subpanels per cluster.
- **KDE/Right Column**: Remains as a single panel per row, as it does not depend on the time-axis.

### 5. Axis Formatting
- Update `format_plot_axes` to handle the new multi-axis structure.
- Ensure x-axis labels and tickmarks are only shown on the bottom-most row of subpanels.
- Ensure y-axis labels and tickmarks are only shown on the left-most column of subpanels.
- Ensure the the y ranges are shared only per-row, not all over the subpanels
- Remove redundant y-axis labels for internal subpanels.
- Promote the title of the [0,0] panel to the title of the whole page

## Technical Details
- **Cluster Widths**: If clusters are $[(t_{0,start}, t_{0,end}), \dots, (t_{N,start}, t_{N,end})]$, the width ratio for `GridSpec` will be $[(t_{0,end}-t_{0,start}), \dots, (t_{N,end}-t_{N,start})]$.
- **Y-Axis Sharing**: Use `sharey=axes[i, 0]` when creating sub-axes for row $i$.

.kilo/legend_relocation_plan.md

deleted100644 → 0
+0 −27
Changes for .kilo/legend_relocation_plan.md: 0 added lines, 27 removed lines.
Original line number Diff line number Diff line
# Plan: Relocate Legends for Overlay and Detrended Rows

## Motivation
Currently, legends for the model overlay and detrended rows are placed within the first subpanel (leftmost column). In multi-cluster/multi-instrument plots, this can clutter the data visualization in the first panel while leaving the right side of the figure underutilized. 

To maintain consistency with the residual row—which uses a dedicated KDE inset axis on the right—the legends for the upper rows should be moved to a similar dedicated space. This improves visual balance and ensures that data in the first subpanel is not obscured by the legend.

## Objectives
1.  **Modify Figure Layout**: Update `FigureLayoutManager` to expand the `GridSpec` by one column. This new rightmost column will be dedicated to legends for the `overlay` and `detrended` rows.
2.  **Implement Legend Axes**: Create `legend_ax` for the overlay and detrended rows in the new column, ensuring `axis('off')` is called to keep them borderless.
3.  **Update Overlay Rendering**: Modify `_render_rv_overlay` and `_render_lc_overlay` in `panel_overlay.py` to:
    *   Remove labels from the primary subpanels (set all to `_nolegend_`).
    *   Use the new dedicated `legend_ax` to render the legend, using `bbox_to_anchor` for precise centering.
4.  **Update Detrended Rendering**: Apply similar logic to the detrended row rendering.
5.  **Verification**: Ensure that the shift in `GridSpec` does not cause overlap between the new legends and the dynamically positioned KDE insets.

## Technical Steps
- [ ] **`PyAETNA/reporting/plot_styling.py`**: 
    - Update `FigureLayoutManager.create()`:
        - Change `GridSpec(self.n_panels, self.n_clusters, ...)` to `GridSpec(self.n_panels, self.n_clusters + 1, ...)`.
        - Add `width_ratios` to the `GridSpec` (e.g., `[1]*self.n_clusters + [0.1]`) to keep the legend column narrow.
        - Instantiate `self.axes['overlay_legend']` and `self.axes['detrended_legend']` using `self.fig.add_subplot(gs[row, self.n_clusters])`.
        - Call `.axis('off')` on these new axes.
- [ ] **`PyAETNA/reporting/panel_overlay.py`**:
    - Update `_render_rv_overlay` and `_render_lc_overlay` to stop using `lbl = j == 0`.
    - Use the `overlay_legend` axis to call `.legend()` with `loc='center left'` and `bbox_to_anchor=(0, 0.5)`.
- [ ] **Detrended Row Logic**: Identify the rendering function for the detrended row and implement the legend relocation to `detrended_legend`.
+8 −0
Changes for PyAETNA/sampling/config/parser/planets.py: 8 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -119,6 +119,8 @@ def parse_planets(config, plModel, plData, n_instruments, free_params, fixed_par
                )
                planet_config['params']['tau'] = tau_names[0]
                planet_config['tau_mode'] = 'direct'
                param_scopes[f'tau_{planet_suffix}'] = tau_scope
                param_groups[f'tau_{planet_suffix}'] = tau_names
            else:
                if tau_ref_value is None:
                    raise ValueError(
@@ -154,6 +156,8 @@ def parse_planets(config, plModel, plData, n_instruments, free_params, fixed_par
                )
                planet_config['params']['delta_tau'] = delta_tau_names[0]
                planet_config['tau_mode'] = 'ref_delta'
                param_scopes[f'tau_{planet_suffix}'] = delta_tau_scope
                param_groups[f'tau_{planet_suffix}'] = delta_tau_names

            period_ref_value = resolve_config_value(planet_cfg.get('period_ref', None), named_priors, f'{key}.period_ref')
            delta_period_value = resolve_config_value(planet_cfg.get('delta_period', None), named_priors, f'{key}.delta_period')
@@ -180,6 +184,8 @@ def parse_planets(config, plModel, plData, n_instruments, free_params, fixed_par
                )
                planet_config['params']['period'] = period_names[0]
                planet_config['period_mode'] = 'direct'
                param_scopes[f'period_{planet_suffix}'] = period_scope
                param_groups[f'period_{planet_suffix}'] = period_names
            else:
                if period_ref_value is None:
                    raise ValueError(
@@ -214,6 +220,8 @@ def parse_planets(config, plModel, plData, n_instruments, free_params, fixed_par
                )
                planet_config['params']['delta_period'] = delta_period_names[0]
                planet_config['period_mode'] = 'ref_delta'
                param_scopes[f'period_{planet_suffix}'] = delta_period_scope
                param_groups[f'period_{planet_suffix}'] = delta_period_names

            _ttv_raw = planet_cfg.get('ttv', False)
            if _ttv_raw is False or _ttv_raw is None:
+7 −0
Changes for PyAETNA/sampling/config/parser/scoping.py: 7 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -108,6 +108,13 @@ def _parse_scoped_param(value, allowed_scopes, n_instruments, param_label):
    if isinstance(value, tuple):
        if 'global' not in allowed_scopes:
            raise ValueError(f"Parameter '{param_label}': global scope not allowed. Allowed: {list(allowed_scopes)}")
        # Plan: If a distribution is provided without a scope keyword, it must be explicitly marked 'global'
        # to be registered as a free parameter.
        raise ValueError(
            f"Parameter '{param_label}': distribution provided without scope keyword. "
            f"To sample this parameter, use the format [global, dist, p1, p2]. "
            f"Got: {value}"
        )
        return 'global', [value]
    
    # String that evaluates to a number (e.g., '-jnp.inf') -> global fixed
+88 −35
Changes for PyAETNA/sampling/engine/logging.py: 88 added lines, 35 removed lines.
Original line number Diff line number Diff line
@@ -136,57 +136,108 @@ def _log_config_summary(param_config, plModel, plData, instrument_names, logger)
    all_params = []
    param_order = param_config.get('param_order', [])

    # Collect tied parameter mappings using the most general approach:
    # 1. Identify all prior__ parameters in free_params and fixed_params
    # 2. Map each to its inline names via param_groups
    # 3. Fall back to planet_configs for direct mappings
    # This approach works for any parameter section as long as param_groups is populated.
    tied_params_map = {}

    # First, identify all prior__ parameters across free and fixed
    all_prior_params = set()
    for param_name in list(free_params.keys()) + list(fixed_params.keys()):
        if isinstance(param_name, str) and param_name.startswith('prior__'):
            all_prior_params.add(param_name)

    # Now map each prior__ parameter to its inline names via param_groups
    param_groups = param_config.get('param_groups', {})
    for group_key, group_names in param_groups.items():
        if not isinstance(group_names, (list, tuple)):
            continue
        
        # Separate priors and inline names in this group
        group_priors = [name for name in group_names if isinstance(name, str) and name.startswith('prior__')]
        group_inlines = [name for name in group_names if isinstance(name, str) and not name.startswith('prior__')]
        
        # Map each prior in this group to all its inline names
        for tied_name in group_priors:
            if tied_name not in tied_params_map:
                tied_params_map[tied_name] = set()
            if group_inlines:
                # Add actual inline parameter names
                for inline_name in group_inlines:
                    tied_params_map[tied_name].add((inline_name, tied_name))
            else:
                # No inline names in group, use the group key as display name
                tied_params_map[tied_name].add((group_key, tied_name))

    # Fallback: check planet_configs for planet-specific parameter ties
    # This ensures backward compatibility and catches any planet params not in param_groups
    planet_configs = param_config.get('planet_configs', [])
    for planet_cfg in planet_configs:
        planet_name = planet_cfg.get('name', '')
        planet_params = planet_cfg.get('params', {})
        for model_param_name, tied_name in planet_params.items():
            if tied_name is None or not isinstance(tied_name, str):
                continue
            if tied_name.startswith('prior__'):
                inline_name = f"{model_param_name}_{planet_name}"
                if tied_name not in tied_params_map:
                    tied_params_map[tied_name] = set()
                tied_params_map[tied_name].add((inline_name, tied_name))

    # Convert to sorted lists for deterministic output
    tied_params_map = {k: sorted(v) for k, v in tied_params_map.items()}

    # Identify canonical priors: parameters that have tied entries
    canonical_priors = set()
    for tied_name in tied_params_map.keys():
        if tied_name.startswith('prior__'):
            canonical_priors.add(tied_name[len('prior__'):])
        else:
            canonical_priors.add(tied_name)

    # Add free parameters in the order specified by param_order
    for name in param_order:
        if name in free_params:
            spec = free_params[name]
            scope = get_param_scope(name)
            all_params.append((name, format_prior(spec), scope, 'free'))
            final_name = f"prior__{name}" if name in canonical_priors else name
            all_params.append((final_name, format_prior(spec), scope, 'free'))

    # Add any free parameters not in param_order (shouldn't happen, but be safe)
    seen_free = {name for name, _, _, _ in all_params if all_params and all_params[-1][3] == 'free'}
    for name, spec in sorted(free_params.items()):
        if name not in seen_free:
            scope = get_param_scope(name)
            all_params.append((name, format_prior(spec), scope, 'free'))
            final_name = f"prior__{name}" if name in canonical_priors else name
            all_params.append((final_name, format_prior(spec), scope, 'free'))

    # Add fixed parameters (sorted)
    for name, value in sorted(fixed_params.items()):
        scope = get_param_scope(name)
        all_params.append((name, value, scope, 'fixed'))

    # Collect tied parameters mapping: tied_canonical_name -> list of (inline_name, ref_name)
    tied_params_map = {}
    gp_configs = param_config.get('gp_configs', {})
    for inst_id, gp_cfg in gp_configs.items():
        inst_name = gp_cfg.get('inst_name', '')
        kernel_specs = gp_cfg.get('kernel_specs', [])
        for spec in kernel_specs:
            gp_name = spec.get('section_name', '')
            param_scopes = spec.get('param_scopes', {})
            param_lookup_names = spec.get('param_lookup_names', {})
            for pname, tied_name in param_lookup_names.items():
                scope = param_scopes.get(pname, 'per_instrument')
                if scope == 'global':
                    inline_name = f"{pname}_{gp_name}"
                else:
                    inline_name = f"{pname}_{gp_name}_{inst_name}"
                
                ref_name = tied_name
                if tied_name.startswith('prior__'):
                    ref_name = tied_name[len('prior__'):]
                
                if tied_name not in tied_params_map:
                    tied_params_map[tied_name] = []
                tied_params_map[tied_name].append((inline_name, ref_name))
        final_name = f"prior__{name}" if name in canonical_priors else name
        all_params.append((final_name, value, scope, 'fixed'))

    # Map all_params to display_params, expanding tied parameters
    display_params = []
    for name, value, scope, fixed_or_free in all_params:
        if name in tied_params_map:
            for inline_name, ref_name in sorted(tied_params_map[name]):
                display_params.append((inline_name, value, scope, fixed_or_free, f"tied to prior @{ref_name}"))
        if name.startswith('prior__'):
            prior_name = name[len('prior__'):]
            
            # Look for tied parameters using both possible keys
            tied_entries = tied_params_map.get(name, []) + tied_params_map.get(prior_name, [])
            
            if tied_entries:
                # Display all tied parameters
                for inline_name, tied_name_full in sorted(set(tied_entries)):
                    # Keep the full 'prior__' prefix in the scope column
                    display_params.append((inline_name, value, scope, fixed_or_free, f", tied to @{tied_name_full}"))
            else:
                # Fallback: show the prior itself with a note (shouldn't happen if setup is correct)
                display_params.append((prior_name, value, scope, fixed_or_free, f", tied to @{name}"))
        else:
            # Regular parameter, not a prior
            display_params.append((name, value, scope, fixed_or_free, ""))

    # Build summary text
@@ -208,8 +259,9 @@ def _log_config_summary(param_config, plModel, plData, instrument_names, logger)
        if free_params_to_print:
            lines.append("FREE PARAMETERS:")
            for name, value, scope, fixed_or_free, note in free_params_to_print:
                note_str = f"  [{note}]" if note else ""
                lines.append(f"  {name:<{max_name_len}}  {str(value):<40}  ({scope}){note_str}")
                # Put the note inside the scope parentheses: (global, tied to @name)
                scope_str = f"({scope}{note})" if note else f"({scope})"
                lines.append(f"  {name:<{max_name_len}}  {str(value):<40}  {scope_str}")
            lines.append("")

        # Print fixed parameters
@@ -217,8 +269,9 @@ def _log_config_summary(param_config, plModel, plData, instrument_names, logger)
        if fixed_params_to_print:
            lines.append("FIXED PARAMETERS:")
            for name, value, scope, fixed_or_free, note in fixed_params_to_print:
                note_str = f"  [{note}]" if note else ""
                lines.append(f"  {name:<{max_name_len}}  {str(value):<40}{note_str}")
                # Put the note inside the scope parentheses: (global, tied to @name)
                scope_str = f"({scope}{note})" if note else f"({scope})"
                lines.append(f"  {name:<{max_name_len}}  {str(value):<40}  {scope_str}")
            lines.append("")

    # Check for SpectralMixture kernels and log component count
Loading