Redundant Refinement Parameter Copying Code Refactoring Plan

[bHimes]

Redundant Refinement Parameter Copying Code Refactoring Plan

This caused me quite a headache when adding refinement parameters to track (that have already been in cistem parameters for a couple years.)

Problem Statement

The same refinement parameter copying code is duplicated across four GUI panel classes, violating the DRY (Don't Repeat Yourself) principle and making maintenance difficult. When new parameters are added (like multi-view data fields), they must be manually added to all four locations.

Current Duplicate Implementations

1. MyRefine3DPanel.cpp (Manual Refinement)

Location: Lines 1707-1754
Function: RefinementManager::ProcessJobResult()

• Copies 25 parameters from result_data array
• Preserves 4 multi-view parameters from input_refinement

2. AutoRefine3dPanel.cpp (Auto Refinement)

Location: Lines 1586-1629
Function: AutoRefine3dManager::ProcessJobResult()

• Identical code structure to Manual refinement
• Same 25 parameters + 4 multi-view parameters

3. AbInitio3DPanel.cpp (Ab Initio)

Location: Lines 2190-2237
Function: AbInitioManager::ProcessJobResult()

• Identical code structure to Manual refinement
• Same 25 parameters + 4 multi-view parameters

4. RefineCTFPanel.cpp (CTF Refinement)

Location: Lines 1394-1403
Function: CTFRefinementManager::ProcessJobResult()

• Subset of parameters (only CTF-related)
• Updates defocus1, defocus2, logp, and score

Code Duplication Analysis

Common Parameters (All Panels Except CTF)

// Position and status
position_in_stack = result_data[1]
image_is_active = result_data[2]

// Alignment parameters
psi = result_data[3]
theta = result_data[4]
phi = result_data[5]
xshift = result_data[6]
yshift = result_data[7]

// CTF parameters
defocus1 = result_data[8]
defocus2 = result_data[9]
defocus_angle = result_data[10]
phase_shift = result_data[11]

// Quality metrics
occupancy = result_data[12]
logp = result_data[13]
sigma = result_data[14]
score = result_data[15]

// Microscope parameters
pixel_size = result_data[17]
microscope_voltage_kv = result_data[18]
microscope_spherical_aberration_mm = result_data[19]

// Beam/Image shifts
beam_tilt_x = result_data[20]
beam_tilt_y = result_data[21]
image_shift_x = result_data[22]
image_shift_y = result_data[23]

// Other
amplitude_contrast = result_data[24]
assigned_subset = result_data[25]

Multi-View Parameters (Preserved from Input)

// Copied from input_refinement to output_refinement
beam_tilt_group
particle_group
pre_exposure
total_exposure

Proposed Solution

Option 1: Method in RefinementResult Class (Recommended)

Add method to RefinementResult class in src/core/refinement.h:

class RefinementResult {
public:
    // Existing members...

    /**
     * Update parameters from worker result array
     * @param result_data Array of floats from worker
     * @param input_result Optional source for preserved parameters
     * @param ctf_only Update only CTF parameters (for CTF refinement)
     */
    void UpdateFromWorkerResult(const float* result_data,
                                const RefinementResult* input_result = nullptr,
                                bool ctf_only = false) {

        if (ctf_only) {
            // CTF refinement only updates these
            defocus1 = result_data[1];
            defocus2 = result_data[2];
            if (result_data[3] != 0) logp = result_data[3];
            if (result_data[4] != 0) score = result_data[4];
            return;
        }

        // Full parameter update
        position_in_stack = long(result_data[1] + 0.5);
        image_is_active = int(result_data[2]);
        psi = result_data[3];
        theta = result_data[4];
        phi = result_data[5];
        xshift = result_data[6];
        yshift = result_data[7];
        defocus1 = result_data[8];
        defocus2 = result_data[9];
        defocus_angle = result_data[10];
        phase_shift = result_data[11];
        occupancy = result_data[12];
        logp = result_data[13];
        sigma = result_data[14];
        score = result_data[15];
        // Skip [16] - not used
        pixel_size = result_data[17];
        microscope_voltage_kv = result_data[18];
        microscope_spherical_aberration_mm = result_data[19];
        beam_tilt_x = result_data[20];
        beam_tilt_y = result_data[21];
        image_shift_x = result_data[22];
        image_shift_y = result_data[23];
        amplitude_contrast = result_data[24];
        assigned_subset = result_data[25];

        // Preserve multi-view data from input if provided
        if (input_result != nullptr) {
            beam_tilt_group = input_result->beam_tilt_group;
            particle_group = input_result->particle_group;
            pre_exposure = input_result->pre_exposure;
            total_exposure = input_result->total_exposure;
        }
    }

    /**
     * Define result array index mapping for documentation
     */
    enum ResultArrayIndex {
        RESULT_CLASS = 0,           // Class number (not in RefinementResult)
        RESULT_POSITION = 1,         // position_in_stack
        RESULT_ACTIVE = 2,           // image_is_active
        RESULT_PSI = 3,              // psi
        RESULT_THETA = 4,            // theta
        RESULT_PHI = 5,              // phi
        RESULT_XSHIFT = 6,           // xshift
        RESULT_YSHIFT = 7,           // yshift
        RESULT_DEFOCUS1 = 8,         // defocus1
        RESULT_DEFOCUS2 = 9,         // defocus2
        RESULT_DEFOCUS_ANGLE = 10,   // defocus_angle
        RESULT_PHASE_SHIFT = 11,     // phase_shift
        RESULT_OCCUPANCY = 12,       // occupancy
        RESULT_LOGP = 13,            // logp
        RESULT_SIGMA = 14,           // sigma
        RESULT_SCORE = 15,           // score
        RESULT_UNUSED = 16,          // Not used
        RESULT_PIXEL_SIZE = 17,      // pixel_size
        RESULT_VOLTAGE = 18,         // microscope_voltage_kv
        RESULT_CS = 19,              // microscope_spherical_aberration_mm
        RESULT_BEAM_TILT_X = 20,     // beam_tilt_x
        RESULT_BEAM_TILT_Y = 21,     // beam_tilt_y
        RESULT_IMAGE_SHIFT_X = 22,   // image_shift_x
        RESULT_IMAGE_SHIFT_Y = 23,   // image_shift_y
        RESULT_AMP_CONTRAST = 24,    // amplitude_contrast
        RESULT_SUBSET = 25,          // assigned_subset
        RESULT_ARRAY_SIZE = 26
    };
};

Option 2: Static Helper Function

Create a static helper in a utility namespace:

namespace RefinementUtils {
    static void UpdateRefinementResultFromWorker(
        RefinementResult& output_result,
        const RefinementResult& input_result,
        const float* result_data,
        bool ctf_only = false);
}

Option 3: Template Method Pattern

Create base class for all refinement managers:

class BaseRefinementManager {
protected:
    virtual void ProcessJobResult(JobResult* result) {
        int current_class = GetClassFromResult(result);
        long current_particle = GetParticleFromResult(result);

        auto& output = GetOutputRefinement()->class_refinement_results[current_class]
                      .particle_refinement_results[current_particle];
        auto& input = GetInputRefinement()->class_refinement_results[current_class]
                     .particle_refinement_results[current_particle];

        output.UpdateFromWorkerResult(result->result_data, &input, IsCTFOnly());

        OnParametersUpdated(current_class, current_particle);
    }

    virtual bool IsCTFOnly() { return false; }
    virtual void OnParametersUpdated(int cls, long particle) {}
};

Refactored Usage Example

Before (MyRefine3DPanel.cpp)

// 35+ lines of manual copying
output_refinement->...->position_in_stack = long(result_to_process->result_data[1] + 0.5);
output_refinement->...->image_is_active = int(result_to_process->result_data[2]);
// ... 23 more parameters ...
output_refinement->...->beam_tilt_group = input_refinement->...->beam_tilt_group;
// ... 3 more multi-view parameters ...

After

// Single line replaces all copying
output_refinement->class_refinement_results[current_class]
    .particle_refinement_results[current_particle]
    .UpdateFromWorkerResult(
        result_to_process->result_data,
        &input_refinement->class_refinement_results[current_class]
            .particle_refinement_results[current_particle]
    );

CTF Refinement After

// CTF-only update
for (int class_counter = 0; class_counter < output_refinement->number_of_classes; class_counter++) {
    output_refinement->class_refinement_results[class_counter]
        .particle_refinement_results[current_particle]
        .UpdateFromWorkerResult(result_to_process->result_data, nullptr, true);
}

Benefits

Maintainability

• Single source of truth for parameter mapping
• Easier to add new parameters - only update one location
• Reduced chance of errors when adding fields

Code Quality

• ~140 lines removed across 4 files
• Better documentation through centralized enum
• Type safety with proper index constants

Testing

• Single test point for parameter copying logic
• Easy to mock for unit testing
• Clear contract for worker result format

Migration Strategy

Phase 1: Add Method (Non-Breaking)

1. Add UpdateFromWorkerResult() to RefinementResult class
2. Add unit tests for the new method
3. Document the result array format

Phase 2: Update Panels (One at a Time)

1. Update MyRefine3DPanel to use new method
2. Test Manual refinement thoroughly
3. Update AutoRefine3dPanel
4. Test Auto refinement
5. Update AbInitio3DPanel
6. Test Ab Initio
7. Update RefineCTFPanel
8. Test CTF refinement

Phase 3: Cleanup

1. Remove old doxygen TODO comments
2. Update developer documentation
3. Add compile-time checks for array size

Potential Issues and Mitigations

Issue 1: Array Index Changes

Risk: Worker programs might change result array layout
Mitigation: Use enum constants, add static_assert checks

Issue 2: Different Panels Need Different Parameters

Risk: CTF panel needs subset, others need full set
Mitigation: Use ctf_only flag or separate methods

Issue 3: Future Parameter Additions

Risk: New parameters need to be added
Mitigation: Well-documented process, single location to update

Testing Checklist

• Unit test for UpdateFromWorkerResult with normal data
• Unit test for UpdateFromWorkerResult with CTF-only
• Unit test for multi-view parameter preservation
• Integration test for Manual refinement
• Integration test for Auto refinement
• Integration test for Ab Initio
• Integration test for CTF refinement
• Regression test for parameter accuracy
• Performance test (ensure no slowdown)

Conclusion

This refactoring eliminates significant code duplication, improves maintainability, and provides a clear path for future enhancements. The centralized parameter copying logic will reduce bugs and make the codebase more maintainable.