diff --git a/wres-config/src/wres/config/yaml/deserializers/EventDetectionDeserializer.java b/wres-config/src/wres/config/yaml/deserializers/EventDetectionDeserializer.java index 54bf9d452..f9260e251 100644 --- a/wres-config/src/wres/config/yaml/deserializers/EventDetectionDeserializer.java +++ b/wres-config/src/wres/config/yaml/deserializers/EventDetectionDeserializer.java @@ -91,27 +91,27 @@ else if ( datasetNode.isArray() ) } if ( parametersNode.has( "start_radius" ) ) { - Duration windowSize = DurationDeserializer.getDuration( reader, - parametersNode, - "start_radius", - DURATION_UNIT ); - parameters.windowSize( windowSize ); + Duration startRadius = DurationDeserializer.getDuration( reader, + parametersNode, + "start_radius", + DURATION_UNIT ); + parameters.startRadius( startRadius ); } if ( parametersNode.has( "half_life" ) ) { - Duration windowSize = DurationDeserializer.getDuration( reader, - parametersNode, - "half_life", - DURATION_UNIT ); - parameters.windowSize( windowSize ); + Duration halfLife = DurationDeserializer.getDuration( reader, + parametersNode, + "half_life", + DURATION_UNIT ); + parameters.halfLife( halfLife ); } if ( parametersNode.has( "minimum_event_duration" ) ) { - Duration windowSize = DurationDeserializer.getDuration( reader, - parametersNode, - "minimum_event_duration", - DURATION_UNIT ); - parameters.windowSize( windowSize ); + Duration minimumEventDuration = DurationDeserializer.getDuration( reader, + parametersNode, + "minimum_event_duration", + DURATION_UNIT ); + parameters.minimumEventDuration( minimumEventDuration ); } } return EventDetectionBuilder.builder() diff --git a/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java b/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java index b9fb4e83d..b504f72ef 100644 --- a/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java +++ b/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java @@ -2435,6 +2435,9 @@ void testDeserializeWithEventDetectionUsingExplicitDatasetAndMethodWithParameter method: regina-ogden parameters: window_size: 1 + start_radius: 2 + half_life: 3 + minimum_event_duration: 4 duration_unit: hours """; @@ -2443,6 +2446,9 @@ void testDeserializeWithEventDetectionUsingExplicitDatasetAndMethodWithParameter EventDetectionParameters parameters = EventDetectionParametersBuilder.builder() .windowSize( java.time.Duration.ofHours( 1 ) ) + .startRadius( java.time.Duration.ofHours( 2 ) ) + .halfLife( java.time.Duration.ofHours( 3 ) ) + .minimumEventDuration( java.time.Duration.ofHours( 4 ) ) .build(); EventDetection eventDetection = EventDetectionBuilder.builder() .datasets( Set.of( EventDetectionDataset.OBSERVED ) ) diff --git a/wres-eventdetection/src/wres/eventdetection/ReginaOgdenEventDetector.java b/wres-eventdetection/src/wres/eventdetection/ReginaOgdenEventDetector.java index 14575850e..517d1cebe 100644 --- a/wres-eventdetection/src/wres/eventdetection/ReginaOgdenEventDetector.java +++ b/wres-eventdetection/src/wres/eventdetection/ReginaOgdenEventDetector.java @@ -2,14 +2,18 @@ import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import java.util.function.Function; import java.util.stream.Collectors; import com.google.protobuf.Timestamp; @@ -186,7 +190,7 @@ private void setSeriesSpecificParameterDefaults( EventDetectionParametersBuilder { builder.halfLife( builder.windowSize() .dividedBy( 10 ) ); - LOGGER.warn( "When performing event detection with the Regina-Ogden method, the half life was " + LOGGER.warn( "When performing event detection with the Regina-Ogden method, the half-life was " + "undefined. However, the window size was defined. The default half life is {}, " + "which is one tenth of the window size. This default may not be appropriate and it " + "is strongly recommended that you set the half life explicitly using the " @@ -197,10 +201,10 @@ private void setSeriesSpecificParameterDefaults( EventDetectionParametersBuilder { builder.halfLife( averageTimestep.multipliedBy( 10 ) ); LOGGER.warn( "When performing event detection with the Regina-Ogden method, the half life was " - + "undefined. The default half life is {}, which is ten times the average time-step " + + "undefined. The default half life is {}, which is ten times the modal time-step " + "associated with the time-series used for event detection. This default may not be " - + "appropriate and it is strongly recommended that you set the half life explicitly using " - + "the 'half_life' parameter.", + + "appropriate and it is strongly recommended that you set the half life explicitly " + + "using the 'half_life' parameter.", builder.halfLife() ); } } @@ -223,10 +227,11 @@ private void setSeriesSpecificParameterDefaults( EventDetectionParametersBuilder { builder.windowSize( averageTimestep.multipliedBy( 100 ) ); LOGGER.warn( "When performing event detection with the Regina-Ogden method, the window wize for " - + "smoothing and detecting trends was undefined. The default window size is {}, which is " - + "one hundred times the average time-step associated with the time-series used for event " - + "detection. This default may not be appropriate and it is strongly recommended that you " - + "set the window size explicitly using the 'window_size' parameter.", + + "smoothing and detecting trends was undefined. The default window size is {}, which " + + "is one hundred times the modal time-step associated with the time-series used for " + + "event detection. This default may not be appropriate and it is strongly " + + "recommended that you set the window size explicitly using the 'window_size' " + + "parameter.", builder.windowSize() ); } } @@ -362,7 +367,7 @@ private TimeSeries detrend( TimeSeries timeSeries, Duration half } /** - * Smoothes a time-series with an exponential moving average. + * Creates an exponential moving average of a time-series, handling missing values. * * @param timeSeries the time-series * @param halfLife the half life @@ -403,7 +408,7 @@ private TimeSeries exponentialMovingAverage( TimeSeries timeSeri double alpha = 1.0 - Math.exp( -Math.log( 2 ) / halfLifeMillis * timestepMillis ); - // Handle NaN + // Current value is missing if ( Double.isNaN( currentValue.getValue() ) ) { // First event, then insert NaN @@ -422,13 +427,23 @@ private TimeSeries exponentialMovingAverage( TimeSeries timeSeri // Decay the weight weight = weight * ( 1.0 - alpha ); } + // Current value is present else { - double smoothedValue = alpha * currentValue.getValue() - + ( 1.0 - alpha ) * lastValue * weight; - lastValue = smoothedValue; - Event smoothedEvent = Event.of( currentValue.getTime(), smoothedValue ); - smoothed.add( smoothedEvent ); + // Last value is missing + if ( Double.isNaN( lastValue ) ) + { + lastValue = currentValue.getValue(); + } + // Current and last value are both present + else + { + double smoothedValue = alpha * currentValue.getValue() + + ( 1.0 - alpha ) * lastValue * weight; + lastValue = smoothedValue; + Event smoothedEvent = Event.of( currentValue.getTime(), smoothedValue ); + smoothed.add( smoothedEvent ); + } weight = weight * ( 1.0 - alpha ) + alpha; } @@ -454,29 +469,38 @@ private long getWindowSizeFromDuration( TimeSeries timeSeries, Duration } /** - * Calculates the average time-step in the series. + * Calculates the modal time-step in the series. * * @param timeSeries the time-series to inspect */ private Duration getAverageTimestep( TimeSeries timeSeries ) { - // Sum the durations between times + + List durations = new ArrayList<>(); Instant lastTime = null; - Duration sum = Duration.ZERO; for ( Event next : timeSeries.getEvents() ) { Instant nextTime = next.getTime(); if ( Objects.nonNull( lastTime ) ) { Duration between = Duration.between( lastTime, nextTime ); - sum = sum.plus( between ); + durations.add( between ); } lastTime = nextTime; } - return sum.dividedBy( timeSeries.getEvents() - .size() ); + return durations.stream() + .collect( Collectors.groupingBy( Function.identity(), + Collectors.counting() ) ) + .entrySet() + .stream() + .max( Map.Entry.comparingByValue() ) + .map( Map.Entry::getKey ) + .orElseThrow( () -> new EventDetectionException( "Insufficient data to calculate the modal " + + "timestep for event detection. The " + + "time-series had the following metadata: " + + timeSeries.getMetadata() ) ); } /**