From f76f1a3acb83f90e64eaf22b26ca3a4c13ea9a27 Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Fri, 30 Aug 2024 19:35:45 +0100 Subject: [PATCH 1/6] Accept date-times in the ISO8601 "basic" format with optional minutes and seconds, #292. --- .../wrds/nwm/DateTimeDeserializer.java | 45 +++++++++++++++++++ .../wrds/nwm/NwmDataPointDeserializer.java | 31 +++++++------ .../wres/reading/wrds/nwm/NwmForecast.java | 2 + .../reading/wrds/nwm/WrdsNwmJsonReader.java | 2 +- .../wrds/nwm/WrdsNwmJsonReaderTest.java | 3 +- 5 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java diff --git a/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java b/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java new file mode 100644 index 0000000000..f129da2e60 --- /dev/null +++ b/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java @@ -0,0 +1,45 @@ +package wres.reading.wrds.nwm; + +import java.io.IOException; +import java.time.Instant; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; + +/** + * Custom deserializer for a datetime string in the ISO8601 "basic" format with optional minutes and seconds. For + * example: 20240830T12Z, 20240830T1200Z and 20240830T120000Z are all acceptable. + * + * @author James Brown + */ +public class DateTimeDeserializer extends JsonDeserializer +{ + @Override + public Instant deserialize( JsonParser jp, DeserializationContext context ) + throws IOException + { + JsonNode node = jp.getCodec() + .readTree( jp ); + + String time; + + // Parse the instant. + if ( node.isTextual() ) + { + time = node.asText(); + } + else + { + throw new IOException( "Could not find a datetime field in the document, which is not allowed." ); + } + + // Lenient formatting in the "basic" ISO8601 format, hours and seconds are optional + DateTimeFormatter formatter = DateTimeFormatter.ofPattern( "yyyyMMdd'T'HH[mm[ss]]'Z'" ) + .withZone( ZoneId.of( "UTC" ) ); + return formatter.parse( time, Instant::from ); + } +} \ No newline at end of file diff --git a/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java b/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java index 286c0bd14a..12e721e2e0 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java +++ b/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java @@ -3,8 +3,6 @@ import java.io.IOException; import java.io.Serial; import java.time.Instant; -import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeFormatterBuilder; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; @@ -22,6 +20,9 @@ public class NwmDataPointDeserializer extends StdDeserializer @Serial private static final long serialVersionUID = 5616289115474402095L; + /** Deserializer for a datetime {@link Instant}. **/ + private static final DateTimeDeserializer INSTANT_DESERIALIZER = new DateTimeDeserializer(); + /** * Creates an instance. * @@ -43,21 +44,25 @@ public NwmDataPointDeserializer() @Override public NwmDataPoint deserialize( JsonParser jp, DeserializationContext ctxt ) throws IOException { - JsonNode node = jp.getCodec().readTree( jp ); + JsonNode node = jp.getCodec() + .readTree( jp ); + + JsonNode timeNode = node.get( "time" ); + JsonParser parser = timeNode.traverse(); + parser.setCodec( jp.getCodec() ); - //Parse the instant. - DateTimeFormatter formatter = new DateTimeFormatterBuilder() - .appendPattern( "uuuuMMdd'T'HHX" ) - .toFormatter(); - Instant instant = formatter.parse( node.get( "time" ).asText(), Instant::from ); + // Parse the instant. + Instant instant = INSTANT_DESERIALIZER.deserialize( parser, ctxt ); - //Parse the value. Note that if the value is null, the node will not be - //null. Rather, isNull will return true. So there is not need to check - //explicitly for null. + // Parse the value. Note that if the value is null, the node will not be + // null. Rather, isNull will return true. So there is no need to check + // explicitly for null. double value = MissingValues.DOUBLE; - if ( !node.get( "value" ).isNull() ) + if ( !node.get( "value" ) + .isNull() ) { - value = Double.parseDouble( node.get( "value" ).asText() ); + value = node.get( "value" ) + .asDouble(); } return new NwmDataPoint( instant, value ); diff --git a/wres-reading/src/wres/reading/wrds/nwm/NwmForecast.java b/wres-reading/src/wres/reading/wrds/nwm/NwmForecast.java index cb6919493a..14198e5111 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/NwmForecast.java +++ b/wres-reading/src/wres/reading/wrds/nwm/NwmForecast.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import org.apache.commons.lang3.builder.ToStringBuilder; /** @@ -28,6 +29,7 @@ public class NwmForecast public NwmForecast( @JsonProperty( "reference_time" ) @JsonFormat( shape = JsonFormat.Shape.STRING, pattern = "uuuuMMdd'T'HHX" ) + @JsonDeserialize( using = DateTimeDeserializer.class ) Instant referenceDatetime, @JsonProperty( "features" ) List nwmFeatures ) diff --git a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmJsonReader.java b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmJsonReader.java index 5ab871a7c9..721099816b 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmJsonReader.java +++ b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmJsonReader.java @@ -158,7 +158,7 @@ private Supplier getTimeSeriesSupplier( DataSource dataSource, // Create a supplier that returns a time-series once complete return () -> { - // Read all of the time-series eagerly on first use: this will still delay any read until a terminal stream + // Read the time-series eagerly on first use: this will still delay any read until a terminal stream // operation pulls from the supplier (which is why we use a reference holder and do not request the // time-series outside of this lambda), but it will then acquire all the time-series eagerly, i.e., now if ( Objects.isNull( timeSeriesTuples.get() ) ) diff --git a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmJsonReaderTest.java b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmJsonReaderTest.java index 62e518dcc5..79031f0005 100644 --- a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmJsonReaderTest.java +++ b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmJsonReaderTest.java @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; @@ -134,7 +133,7 @@ void testReadObservationsFromStreamResultsInOneTimeSeries() throws IOException Stream tupleStream = reader.read( this.fakeSource, inputStream ) ) { List> actual = tupleStream.map( TimeSeriesTuple::getSingleValuedTimeSeries ) - .collect( Collectors.toList() ); + .toList(); TimeSeriesMetadata metadata = TimeSeriesMetadata.of( Map.of( ReferenceTimeType.T0, Instant.parse( "2020-01-12T00:00:00Z" ) ), From 2d3ffcb86d4a54b4b9049b58b658ee633efe544a Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Tue, 3 Sep 2024 17:57:03 +0100 Subject: [PATCH 2/6] Request WRDS National Water Model time-series using date-times in the ISO8601 "basic" format with minutes and seconds, when declared, #292. --- .../reading/wrds/ahps/WrdsAhpsReader.java | 6 +- .../wrds/nwm/DateTimeDeserializer.java | 5 +- .../wrds/nwm/NwmDataPointDeserializer.java | 3 +- .../wres/reading/wrds/nwm/WrdsNwmReader.java | 55 +++++++---- .../reading/wrds/nwm/WrdsNwmReaderTest.java | 91 ++++++++++++++++++- 5 files changed, 134 insertions(+), 26 deletions(-) diff --git a/wres-reading/src/wres/reading/wrds/ahps/WrdsAhpsReader.java b/wres-reading/src/wres/reading/wrds/ahps/WrdsAhpsReader.java index 460927c2cb..4f10b70e54 100644 --- a/wres-reading/src/wres/reading/wrds/ahps/WrdsAhpsReader.java +++ b/wres-reading/src/wres/reading/wrds/ahps/WrdsAhpsReader.java @@ -586,9 +586,11 @@ private Map createWrdsAhpsUrlParameters( Pair } urlParameters.put( timeTag, - "[" + dateRange.getLeft().toString() + "[" + dateRange.getLeft() + .toString() + "," - + dateRange.getRight().toString() + + dateRange.getRight() + .toString() + "]" ); return Collections.unmodifiableMap( urlParameters ); diff --git a/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java b/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java index f129da2e60..29fb7d49c9 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java +++ b/wres-reading/src/wres/reading/wrds/nwm/DateTimeDeserializer.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.time.Instant; -import java.time.ZoneId; import java.time.format.DateTimeFormatter; import com.fasterxml.jackson.core.JsonParser; @@ -10,6 +9,8 @@ import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonNode; +import wres.reading.ReaderUtilities; + /** * Custom deserializer for a datetime string in the ISO8601 "basic" format with optional minutes and seconds. For * example: 20240830T12Z, 20240830T1200Z and 20240830T120000Z are all acceptable. @@ -39,7 +40,7 @@ public Instant deserialize( JsonParser jp, DeserializationContext context ) // Lenient formatting in the "basic" ISO8601 format, hours and seconds are optional DateTimeFormatter formatter = DateTimeFormatter.ofPattern( "yyyyMMdd'T'HH[mm[ss]]'Z'" ) - .withZone( ZoneId.of( "UTC" ) ); + .withZone( ReaderUtilities.UTC ); return formatter.parse( time, Instant::from ); } } \ No newline at end of file diff --git a/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java b/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java index 12e721e2e0..3f1c906234 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java +++ b/wres-reading/src/wres/reading/wrds/nwm/NwmDataPointDeserializer.java @@ -13,7 +13,8 @@ /** * Custom deserializer to allow for handling a null value in a NWM data point. - * @author Hank.Herr + * @author Hank Herr + * @author James Brown */ public class NwmDataPointDeserializer extends StdDeserializer { diff --git a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java index b3e8367024..4d94d07913 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java +++ b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java @@ -8,7 +8,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.time.Instant; -import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.time.temporal.TemporalAdjusters; @@ -256,10 +255,14 @@ private Stream read( DataSource dataSource, EvaluationDeclarati // The chunked features List> featureBlocks = ListUtils.partition( features, this.getFeatureChunkSize() ); + LOGGER.debug( "Will request data for these feature chunks: {}.", featureBlocks ); + // Date ranges Set> dateRanges = WrdsNwmReader.getWeekRanges( declaration, dataSource ); - // Combine the features and date ranges to form the chunk boundaries + LOGGER.debug( "Will request data for these datetime chunks: {}.", dateRanges ); + + // Combine the features and date ranges to form the overall chunk boundaries Set, Pair>> chunks = new HashSet<>(); for ( List nextFeatures : featureBlocks ) { @@ -495,7 +498,7 @@ private static Set> getWeekRanges( EvaluationDeclaration ZonedDateTime latest = dates.maximum() .atZone( ReaderUtilities.UTC ); - LOGGER.debug( "Given {} parsed {} for latest.", + LOGGER.debug( "Given {} calculated {} for latest.", dates.maximum(), latest ); @@ -614,8 +617,12 @@ private URI getUriForChunk( URI baseUri, use ); } - return ReaderUtilities.getUriWithParameters( uriWithLocation, - wrdsParameters ); + URI uri = ReaderUtilities.getUriWithParameters( uriWithLocation, + wrdsParameters ); + + LOGGER.debug( "Will request time-series data with this URI: {}.", uri ); + + return uri; } /** @@ -650,29 +657,45 @@ private Map createWrdsNwmUrlParameters( Pair r // This will override the parameter added above. urlParameters.putAll( additionalParameters ); - String leftWrdsFormattedDate = WrdsNwmReader.iso8601TruncatedToHourFromInstant( range.getLeft() ); - String rightWrdsFormattedDate = WrdsNwmReader.iso8601TruncatedToHourFromInstant( range.getRight() ); + Pair wrdsFormattedDates = WrdsNwmReader.toBasicISO8601String( range.getLeft(), + range.getRight() ); urlParameters.put( "reference_time", - "(" + leftWrdsFormattedDate + "(" + wrdsFormattedDates.getLeft() + "," - + rightWrdsFormattedDate + + wrdsFormattedDates.getRight() + "]" ); - urlParameters.put( "validTime", "all" ); + urlParameters.put( "valid_time", "all" ); return Collections.unmodifiableMap( urlParameters ); } /** - * The WRDS NWM API uses a concise ISO-8601 format rather than RFC3339 instant. - * @param instant the instance + * The WRDS NWM API uses the basic ISO-8601 format for the date range. + * @param left the instant + * @param right the right instant * @return the ISO-8601 instant string */ - private static String iso8601TruncatedToHourFromInstant( Instant instant ) + private static Pair toBasicISO8601String( Instant left, Instant right ) { - String dateFormat = "uuuuMMdd'T'HHX"; + String dateFormat = "yyyyMMdd'T'HH'Z'"; + + ZonedDateTime zonedLeft = left.atZone( ReaderUtilities.UTC ); + ZonedDateTime zonedRight = right.atZone( ReaderUtilities.UTC ); + + if ( zonedLeft.getMinute() > 0 + || zonedLeft.getSecond() > 0 + || zonedRight.getMinute() > 0 + || zonedRight.getSecond() > 0 ) + { + dateFormat = "yyyyMMdd'T'HHmmss'Z'"; + } + DateTimeFormatter formatter = DateTimeFormatter.ofPattern( dateFormat ) - .withZone( ZoneId.of( "UTC" ) ); - return formatter.format( instant ); + .withZone( ReaderUtilities.UTC ); + String leftString = formatter.format( left ); + String rightString = formatter.format( right ); + + return Pair.of( leftString, rightString ); } /** diff --git a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java index c217992591..e17a0579e6 100644 --- a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java +++ b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java @@ -385,19 +385,19 @@ void testReadReturnsThreeChunkedForecastTimeSeries() new Parameter( "reference_time", "(20220102T00Z,20220109T00Z]" ), new Parameter( "forecast_type", "deterministic" ), - new Parameter( "validTime", "all" ) ); + new Parameter( "valid_time", "all" ) ); Parameters parametersTwo = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20220109T00Z,20220116T00Z]" ), new Parameter( "forecast_type", "deterministic" ), - new Parameter( "validTime", "all" ) ); + new Parameter( "valid_time", "all" ) ); Parameters parametersThree = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20220116T00Z,20220123T00Z]" ), new Parameter( "forecast_type", "deterministic" ), - new Parameter( "validTime", "all" ) ); + new Parameter( "valid_time", "all" ) ); this.mockServer.when( HttpRequest.request() .withPath( FORECAST_PATH ) @@ -502,7 +502,6 @@ void testReadReturnsThreeChunkedForecastTimeSeries() VerificationTimes.exactly( 1 ) ); } - /** * Tests for an expected exception and not an unexpected one. See #109238. */ @@ -574,4 +573,86 @@ void testReadDoesNotThrowClassCastExceptionWhenChunkingFeatures() } ); } -} + @Test + void testReadRequestsDateRangeWithMinutesAndSeconds() + { + this.mockServer.when( HttpRequest.request() + .withPath( ANALYSIS_PATH ) + .withMethod( GET ) ) + .respond( HttpResponse.response( ANALYSIS_RESPONSE ) ); + + URI fakeUri = URI.create( "http://localhost:" + + this.mockServer.getLocalPort() + + "/api/v1/nwm/ops/analysis_assim/" + + ANALYSIS_PARAMS ); + + Source fakeDeclarationSource = SourceBuilder.builder() + .uri( fakeUri ) + .sourceInterface( SourceInterface.WRDS_NWM ) + .build(); + + Dataset dataset = DatasetBuilder.builder() + .sources( List.of( fakeDeclarationSource ) ) + .variable( VariableBuilder.builder() + .name( "streamflow" ) + .build() ) + .build(); + + DataSource fakeSource = DataSource.of( DataSource.DataDisposition.JSON_WRDS_NWM, + fakeDeclarationSource, + dataset, + Collections.emptyList(), + fakeUri, + DatasetOrientation.LEFT, + null ); + + SystemSettings systemSettings = Mockito.mock( SystemSettings.class ); + Mockito.when( systemSettings.getMaximumWebClientThreads() ) + .thenReturn( 6 ); + Mockito.when( systemSettings.getPoolObjectLifespan() ) + .thenReturn( 30_000 ); + + // Create a declaration with a short period of valid dates to create one chunked request where the first and + // last datetimes both contain minutes and seconds + Instant minimum = Instant.parse( "2024-09-01T00:13:00Z" ); + Instant maximum = Instant.parse( "2024-09-03T00:27:59Z" ); + TimeInterval interval = TimeIntervalBuilder.builder() + .minimum( minimum ) + .maximum( maximum ) + .build(); + Set geometries + = Set.of( GeometryTuple.newBuilder() + .setLeft( Geometry.newBuilder() + .setName( Integer.toString( NWM_FEATURE_ID ) ) ) + .build() ); + Features features = FeaturesBuilder.builder() + .geometries( geometries ) + .build(); + EvaluationDeclaration declaration = EvaluationDeclarationBuilder.builder() + .validDates( interval ) + .features( features ) + .build(); + + WrdsNwmReader reader = WrdsNwmReader.of( declaration, systemSettings ); + + Parameters parametersOne = new Parameters( new Parameter( "valid_time", "all" ), + new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), + new Parameter( "reference_time", + "(20240901T000000Z,20240903T002759Z]" ), + new Parameter( "forecast_type", "deterministic" ) ); + + try ( Stream tupleStream = reader.read( fakeSource ) ) + { + List> actual = tupleStream.map( TimeSeriesTuple::getSingleValuedTimeSeries ) + .toList(); + + assertEquals( 1, actual.size() ); + + // One request made with parameters one + this.mockServer.verify( request().withMethod( GET ) + .withPath( ANALYSIS_PATH ) + .withQueryStringParameters( parametersOne ), + VerificationTimes.exactly( 1 ) ); + } + } +} \ No newline at end of file From 296021e88be3a0dc8ccedde3553565f682e11ac7 Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Tue, 3 Sep 2024 20:09:30 +0100 Subject: [PATCH 3/6] Remove the redundant valid_time parameter from WRDS NWM requests, #292. --- .../src/wres/reading/wrds/nwm/WrdsNwmReader.java | 1 - .../wres/reading/wrds/nwm/WrdsNwmReaderTest.java | 12 ++++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java index 4d94d07913..d89e80d2da 100644 --- a/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java +++ b/wres-reading/src/wres/reading/wrds/nwm/WrdsNwmReader.java @@ -664,7 +664,6 @@ private Map createWrdsNwmUrlParameters( Pair r + "," + wrdsFormattedDates.getRight() + "]" ); - urlParameters.put( "valid_time", "all" ); return Collections.unmodifiableMap( urlParameters ); } diff --git a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java index e17a0579e6..b5f1d7aab4 100644 --- a/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java +++ b/wres-reading/test/wres/reading/wrds/nwm/WrdsNwmReaderTest.java @@ -384,20 +384,17 @@ void testReadReturnsThreeChunkedForecastTimeSeries() Parameters parametersOne = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20220102T00Z,20220109T00Z]" ), - new Parameter( "forecast_type", "deterministic" ), - new Parameter( "valid_time", "all" ) ); + new Parameter( "forecast_type", "deterministic" ) ); Parameters parametersTwo = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20220109T00Z,20220116T00Z]" ), - new Parameter( "forecast_type", "deterministic" ), - new Parameter( "valid_time", "all" ) ); + new Parameter( "forecast_type", "deterministic" ) ); Parameters parametersThree = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20220116T00Z,20220123T00Z]" ), - new Parameter( "forecast_type", "deterministic" ), - new Parameter( "valid_time", "all" ) ); + new Parameter( "forecast_type", "deterministic" ) ); this.mockServer.when( HttpRequest.request() .withPath( FORECAST_PATH ) @@ -635,8 +632,7 @@ void testReadRequestsDateRangeWithMinutesAndSeconds() WrdsNwmReader reader = WrdsNwmReader.of( declaration, systemSettings ); - Parameters parametersOne = new Parameters( new Parameter( "valid_time", "all" ), - new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), + Parameters parametersOne = new Parameters( new Parameter( "proj", "UNKNOWN_PROJECT_USING_WRES" ), new Parameter( "reference_time", "(20240901T000000Z,20240903T002759Z]" ), new Parameter( "forecast_type", "deterministic" ) ); From 50bc3d300f7cd0eb3ca61b30b1f392a3dda94557 Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Wed, 4 Sep 2024 13:52:45 +0100 Subject: [PATCH 4/6] Validate the feature name orientation (feature_name_from) declared alongside thresholds, #287. --- .../config/yaml/DeclarationValidator.java | 52 ++++++++++++++++--- .../config/yaml/DeclarationValidatorTest.java | 30 +++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/wres-config/src/wres/config/yaml/DeclarationValidator.java b/wres-config/src/wres/config/yaml/DeclarationValidator.java index e3cf325432..585f90352f 100644 --- a/wres-config/src/wres/config/yaml/DeclarationValidator.java +++ b/wres-config/src/wres/config/yaml/DeclarationValidator.java @@ -122,7 +122,9 @@ public class DeclarationValidator private static final String THE_EVALUATION_REQUESTED_THE_SAMPLING_UNCERTAINTY = "The evaluation requested the 'sampling_uncertainty' "; /** Re-used string. */ - private static final String AND_TRY_AGAIN = "and try again."; + private static final String TRY_AGAIN = "try again."; + /** Re-used string. */ + private static final String AND_TRY_AGAIN = "and " + TRY_AGAIN; /** * Performs validation against the schema, followed by "business-logic" validation if there are no schema @@ -1246,7 +1248,7 @@ private static List covariateFiltersAreValid( EvaluationD + ( variables.size() - named.size() ) + ". Please fix the 'minimum' and/or " + "'maximum' value associated with these covariates and " - + "try again." ) + + TRY_AGAIN ) .build(); events.add( event ); } @@ -1542,7 +1544,7 @@ else if ( Objects.isNull( dataset.rescaleFunction() ) .getFunction() + "'. If this is incorrect, please add the correct " + "'rescale_function' for the covariate dataset and " - + "try again." ) + + TRY_AGAIN ) .build(); events.add( event ); } @@ -2365,9 +2367,15 @@ private static List thresholdsAreValid( EvaluationDeclara List events = new ArrayList<>( DeclarationValidator.thresholdSourcesAreValid( declaration ) ); + // Check that value thresholds include units List valueThresholds = DeclarationValidator.valueThresholdsIncludeUnits( declaration ); events.addAll( valueThresholds ); + // Check that the feature orientation is consistent with other declaration + List featureThresholds = + DeclarationValidator.thresholdFeatureNameFromIsValid( declaration ); + events.addAll( featureThresholds ); + return Collections.unmodifiableList( events ); } @@ -2416,6 +2424,36 @@ private static List valueThresholdsIncludeUnits( Evaluati return Collections.unmodifiableList( events ); } + /** + * Checks that the {@code feature_name_from} declaration associated with thresholds is valid, specifically that a + * baseline dataset exists when a baseline orientation is declared. + * @param declaration the declaration + * @return the validation events encountered + */ + private static List thresholdFeatureNameFromIsValid( EvaluationDeclaration declaration ) + { + List events = new ArrayList<>(); + + Set thresholds = DeclarationUtilities.getInbandThresholds( declaration ); + if ( thresholds.stream() + .anyMatch( t -> t.featureNameFrom() == DatasetOrientation.BASELINE ) + && !DeclarationUtilities.hasBaseline( declaration ) ) + { + EvaluationStatusEvent event + = EvaluationStatusEvent.newBuilder() + .setStatusLevel( StatusLevel.ERROR ) + .setEventMessage( "Discovered one or more thresholds with a " + + "'feature_name_from' of 'baseline', but the evaluation " + + "does not declare a 'baseline' dataset. Please fix the " + + "'feature_name_from' or declare a 'baseline' dataset and " + + TRY_AGAIN ) + .build(); + events.add( event ); + } + + return Collections.unmodifiableList( events ); + } + /** * Checks that the sampling uncertainty declaration is valid. * @param declaration the declaration @@ -2690,13 +2728,13 @@ private static List thresholdSourceIsValid( ThresholdSour EvaluationStatusEvent event = EvaluationStatusEvent.newBuilder() .setStatusLevel( StatusLevel.ERROR ) - .setEventMessage( "The 'threshold_service' declaration requested that " + .setEventMessage( "The 'threshold_sources' declaration requested that " + "feature names with an orientation of '" + DatasetOrientation.BASELINE + "' are used to correlate features with thresholds, but " + "no 'baseline' dataset was discovered. Please add a " + "'baseline' dataset or fix the 'feature_name_from' " - + "in the 'threshold_service' declaration." ) + + "in the 'threshold_sources' declaration." ) .build(); events.add( event ); @@ -2714,7 +2752,7 @@ private static List thresholdSourceIsValid( ThresholdSour EvaluationStatusEvent event = EvaluationStatusEvent.newBuilder() .setStatusLevel( StatusLevel.ERROR ) - .setEventMessage( "Discovered declaration for a 'threshold_service', which " + .setEventMessage( "Discovered declaration of 'threshold_sources', which " + "requests thresholds whose feature names have an " + "orientation of '" + DatasetOrientation.BASELINE @@ -2723,7 +2761,7 @@ private static List thresholdSourceIsValid( ThresholdSour + " feature(s) were discovered with a missing '" + DatasetOrientation.BASELINE + "' feature name. Please fix the 'feature_name_from' in " - + "the 'threshold_service' declaration or supply fully " + + "the 'threshold_sources' declaration or supply fully " + "composed feature tuples with an appropriate feature " + "for the '" + DatasetOrientation.BASELINE diff --git a/wres-config/test/wres/config/yaml/DeclarationValidatorTest.java b/wres-config/test/wres/config/yaml/DeclarationValidatorTest.java index a0ffb25c08..0f538d92f3 100644 --- a/wres-config/test/wres/config/yaml/DeclarationValidatorTest.java +++ b/wres-config/test/wres/config/yaml/DeclarationValidatorTest.java @@ -1614,6 +1614,36 @@ void testFeaturefulThresholdsNotCorrelatedWithFeaturesToEvaluateProducesErrors() ); } + @Test + void testFeaturefulThresholdsWithFeatureNameFromBaselineAndNoBaselineProducesError() + { + Geometry featureFoo = Geometry.newBuilder() + .setName( "foo" ) + .build(); + Threshold one = Threshold.newBuilder() + .setLeftThresholdValue( 1.0 ) + .build(); + wres.config.yaml.components.Threshold wrappedOne = ThresholdBuilder.builder() + .threshold( one ) + .feature( featureFoo ) + .featureNameFrom( DatasetOrientation.BASELINE ) + .type( ThresholdType.VALUE ) + .build(); + EvaluationDeclaration declaration = + EvaluationDeclarationBuilder.builder() + .left( this.defaultDataset ) + .right( this.defaultDataset ) + .thresholds( Set.of( wrappedOne ) ) + .build(); + + List events = DeclarationValidator.validate( declaration ); + + assertTrue( DeclarationValidatorTest.contains( events, + "Please fix the 'feature_name_from' or declare a " + + "'baseline' dataset", + StatusLevel.ERROR ) ); + } + @Test void testFeaturefulThresholdsAndNoFeaturesProducesWarning() { From 4ab1243da75d0c9f44240923670eddf9af948e58 Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:58:23 +0100 Subject: [PATCH 5/6] Towards consistent logging in all contexts, #290. --- dist/lib/conf/logback.xml | 11 +++++------ wres-eventsbroker/dist/lib/conf/logback.xml | 5 +++-- wres-tasker/dist/lib/conf/logback.xml | 10 ++++------ wres-vis/dist/lib/conf/logback.xml | 10 ++++------ wres-worker/dist/lib/conf/inner_logback.xml | 10 ++++------ wres-worker/dist/lib/conf/logback.xml | 10 ++++------ wres-writing/dist/lib/conf/logback.xml | 10 ++++------ 7 files changed, 28 insertions(+), 38 deletions(-) diff --git a/dist/lib/conf/logback.xml b/dist/lib/conf/logback.xml index 26ae49d39f..2b1c3d8602 100644 --- a/dist/lib/conf/logback.xml +++ b/dist/lib/conf/logback.xml @@ -1,5 +1,4 @@ - @@ -23,13 +22,13 @@ UTF-8 - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-eventsbroker/dist/lib/conf/logback.xml b/wres-eventsbroker/dist/lib/conf/logback.xml index 45a239121e..0ec63eca51 100644 --- a/wres-eventsbroker/dist/lib/conf/logback.xml +++ b/wres-eventsbroker/dist/lib/conf/logback.xml @@ -2,8 +2,9 @@ - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-tasker/dist/lib/conf/logback.xml b/wres-tasker/dist/lib/conf/logback.xml index 10db181956..4945f398be 100644 --- a/wres-tasker/dist/lib/conf/logback.xml +++ b/wres-tasker/dist/lib/conf/logback.xml @@ -1,5 +1,4 @@ - @@ -24,13 +23,12 @@ true - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - @@ -38,7 +36,7 @@ - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-vis/dist/lib/conf/logback.xml b/wres-vis/dist/lib/conf/logback.xml index 474a2616e6..604a998679 100644 --- a/wres-vis/dist/lib/conf/logback.xml +++ b/wres-vis/dist/lib/conf/logback.xml @@ -1,5 +1,4 @@ - @@ -22,13 +21,12 @@ 2048MB - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-worker/dist/lib/conf/inner_logback.xml b/wres-worker/dist/lib/conf/inner_logback.xml index 51ee187f15..72de5fd603 100644 --- a/wres-worker/dist/lib/conf/inner_logback.xml +++ b/wres-worker/dist/lib/conf/inner_logback.xml @@ -1,6 +1,5 @@ - @@ -26,13 +25,12 @@ UTF-8 - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-worker/dist/lib/conf/logback.xml b/wres-worker/dist/lib/conf/logback.xml index 2055f2b3b9..d1bd77ceda 100644 --- a/wres-worker/dist/lib/conf/logback.xml +++ b/wres-worker/dist/lib/conf/logback.xml @@ -1,5 +1,4 @@ - true @@ -23,13 +22,12 @@ true - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - @@ -37,7 +35,7 @@ - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} diff --git a/wres-writing/dist/lib/conf/logback.xml b/wres-writing/dist/lib/conf/logback.xml index 7aa0d5cc42..8e3bec874f 100644 --- a/wres-writing/dist/lib/conf/logback.xml +++ b/wres-writing/dist/lib/conf/logback.xml @@ -1,5 +1,4 @@ - @@ -22,13 +21,12 @@ 2048MB - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger - %msg%n%exception{full} + + + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} - - %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %level %logger{0} %msg%n%exception{full} + %d{yyyy-MM-dd'T'HH:mm:ss.SSSZ} %X{pid} [%thread] %level %logger{0} - %msg%n%exception{full} From ec464f8578608555b9635d974765a8ef620660eb Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Thu, 5 Sep 2024 15:10:12 +0100 Subject: [PATCH 6/6] Update README.md Fix a typo in the README. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 146577e383..a4737b3091 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ value_date,variable_name,location,measurement_unit,value 1985-06-01T14:00:00Z,streamflow,myLocation,CMS,22.0 ``` -* Create a file `observation.csv` with the following content: +* Create a file `observations.csv` with the following content: ``` value_date,variable_name,location,measurement_unit,value @@ -73,4 +73,4 @@ cd build/install/wres/ * Execute your project ``` bin/wres myEvaluation.yml -``` \ No newline at end of file +```