WWZ statistic goes to zero in the presence of gaps

[dbenn]

See https://sourceforge.net/p/vstar/bugs-and-features/328/

This bug goes back a long way, to 2018 and an interaction with Brian Kloppenborg who reported the problem during NACAA 2012. ChangeLog.txt has made reference to this issue since then. Here's what Brian had to say:

"I have found an annoying bug in the WWZ algorithm that leads to premature termination of the WWZ algorithm. This happens both in WinWWZ and in VStar. Steps to reproduce:

1. Load the V_Vstar.[csv|txt] file into VStar or WinWWZ
2. Choose a period range from 20 to 300 days in 0.5 day steps. Decay of 0.0125
3. Run it.
   VStar will terminate after the first block of data. WinWWZ does better, but still appears to terminate before the data ends."
   The data file V_Vstar.txt is attached.

Brian also reported the following which may or may not be related:

"I also found a performance issue in VStar. In both VStar and WinWWZ, select JD 2446000-2455000 and run the WWZ with the aforementioned parameters except crank the decay down to 0.0005. This process is SIGNIFICANTLY faster in WinWWZ than it is in VStar on my machine (I'm even running Windows in a Virtual Machine)."

[dbenn]

So, there are 2 separate issues here but I include them both for completeness as per the original issue when VStar code was in SourceForge:

• premature termination of the WWZ algorithm due to large gaps in a dataset
• a performance issue when the decay value is small

[dbenn]

There is also this WWZ issue which is different to either of the above: #134

[dbenn]

Test data file referred to by Brian K above: V_Vstar.txt

[dbenn]

Test data file converted to simple file format as CSV: V_VStar.csv

[dbenn]

An interaction with Cursor suggested a fix to WeightedWaveletZTransform.java on line 523, adding this line:

dvarw = 0.0; // Reset variance accumulator for each frequency

which:

Ensures the variance accumulator is reset for each frequency/tau combination

WWZ_gap_analysis.md

This one change does fix the problem observed with the supplied data file. Other testing looks promising too.

However, the existing unit test code fails due to the fact that values after gaps are no longer zero as the algorithm is applied. All values other than tau (time) are impacted to some degree.

[dbenn]

Also commented out dweight2 and dfre init to zero because they are set to values further down in the method before otherwise first being used.

[dbenn]

The result for T UMi (user manual p98) is the same after the fix, at least by inspection.

The unit test also uses T UMi data.

[dbenn]

For the test file and WWZ input parameters we have:

Before

After

[dbenn]

Recovery after time periods with no data is the key thing to notice here.

[dbenn]

Here is the analysis from Cursor:

Code Analysis

Location

src/org/aavso/tools/vstar/util/period/wwz/WeightedWaveletZTransform.java

Critical Issues Identified

Issue 1: dvarw Not Reset Per Frequency/Tau ⚠️ PRIMARY BUG

Location: Lines 487, 525, 544, 580, 586

Problem:

• dvarw is initialized once at line 487: dvarw = 0.0;
• It accumulates values across ALL tau and frequency iterations (line 544: dvarw = dvarw + (dxw * dx[idat]);)
• It's never reset for each frequency/tau combination
• Note: dweight2 IS correctly reset at line 525 (inside frequency loop), but dvarw is not
• This causes incorrect variance calculations, especially after gaps

Impact: When processing data after a gap, dvarw contains stale accumulated values from all previous tau/frequency combinations, leading to:

• Incorrect variance normalization (line 580)
• Wrong variance after mean subtraction (line 586)
• Incorrect denominator in WWZ calculation (line 612: dpowz = ... / (dvarw - dpower))
• This can cause division by zero, negative denominators, or completely wrong WWZ values

Issue 2: Gap Handling Logic in Data Iteration Loop

Location: Lines 527-552

Current Logic:

for (idat = nstart; idat <= numdat; idat++) {
    dz = domega * (dt[idat] - dtau);
    dweight = Math.exp(-1.0 * dcon * dz * dz);
    if (dweight > 1.0e-9) {
        // Accumulate statistics
    } else if (dz > 0.0) {
        break;  // Data point is after tau, weights will only get smaller
    } else {
        nstart = idat + 1;  // Data point is before tau, skip it
    }
}

Problem Scenarios:

1. Tau in the middle of a large gap:

   • Early data points (before gap): dz < 0, dweight very small → nstart updated
   • Gap region: No data points
   • Late data points (after gap): dz > 0, dweight very small → break immediately
   • Result: No data accumulated → dweight2 = 0 → dneff = 0 → dpowz = 0

2. Tau after a large gap:

   • All data points before gap: dz < 0, dweight very small → nstart updated repeatedly
   • When reaching tau: If tau is far from any data, dweight remains small
   • Result: May accumulate insufficient data → dneff <= 3.0 → dpowz = 0

3. nstart reset behavior:

   • Line 501: nstart = 1; resets for each tau
   • Line 550: nstart = idat + 1; updates within frequency loop
   • This means nstart optimization only helps within a single tau's frequency iterations
   • For each new tau, we start from the beginning again

Issue 3: Insufficient Data Detection

Location: Lines 563-567, 568-623

Problem:

• If dweight2 == 0.0 (no significant weights), dneff = 0.0
• If dneff <= 3.0, the algorithm sets dpowz = 0.0 (line 616)
• This is correct behavior, but the root cause is that gaps prevent data accumulation

However: The algorithm doesn't distinguish between:

• Legitimate zero WWZ (no periodic signal)
• Zero WWZ due to gap-induced data starvation

Root Cause Summary

The primary issue is Issue 1: dvarw is not reset for each frequency/tau combination. This causes:

1. Incorrect variance calculations when processing data after gaps
2. Stale variance values contaminating calculations
3. Potential division by zero or incorrect denominator values in WWZ calculation (line 612)

Secondary issues:

• The gap handling logic correctly breaks early when data is too far from tau, but this can lead to zero statistics when tau is in or near gaps
• The nstart optimization doesn't persist across tau values, reducing its effectiveness

Recommended Fixes

Fix 1: Reset dvarw for Each Frequency ⚠️ CRITICAL FIX

Location: Line 525 (alongside dweight2 reset)

Current code (line 525):

dweight2 = 0.0;

Should be:

dweight2 = 0.0;
dvarw = 0.0;  // Reset variance accumulator for each frequency

Rationale:

• dweight2 is correctly reset here for each frequency
• dvarw should follow the same pattern since it's also accumulated during the data loop
• This ensures each frequency/tau combination starts with a clean variance accumulator
• Prevents stale values from contaminating calculations after gaps

Alternative: Remove dvarw = 0.0; from line 487 and add it at line 525 to be consistent with dweight2.

Fix 3: Improve Gap Handling

Consider implementing a more sophisticated gap detection mechanism that:

• Detects when tau is in a gap region
• Uses data from both sides of the gap if within reasonable distance
• Provides better handling when tau is near gap boundaries

References

• SourceForge Ticket #327: fixed case where no mean series source has been determined, e.g… #328: https://sourceforge.net/p/vstar/bugs-and-features/328/
• GitHub Issue Fix WWZ infinities #134: Fix WWZ infinities #134
• ChangeLog.txt line 21: "WWZ is brittle in the presence of gaps in the data"

[dbenn]

Note that the narrative above appears to focus on fixes for problems 1 and 3.

For Fix 1, need to verify that any changes to the existing T UMi unit test case make sense.