Skip to content

Commit

Permalink
Merge pull request #98 from GobySoft/4.0-fix-resolution-exception-reg…
Browse files Browse the repository at this point in the history 
…ression

Fix regression enforcing min/max to be a multiple of the resolution when this wasn't enforced previously for the precision
  • Loading branch information
@tsaubergine
tsaubergine authored Mar 21, 2023
2 parents b14a2b6 + 1135df3 commit 3ce4696
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 84 deletions.
21 changes: 17 additions & 4 deletions src/codecs2/field_codec_default.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,24 @@ namespace dccl

// allowable epsilon for min / max to diverge from nearest quantile
const double min_max_eps = 1e-10;
bool min_multiple_of_res = std::abs(quantize(min(), resolution()) - min()) < min_max_eps;
bool max_multiple_of_res = std::abs(quantize(max(), resolution()) - max()) < min_max_eps;
if(FieldCodecBase::dccl_field_options().has_resolution())
{
// ensure that max and min are multiples of the resolution chosen
FieldCodecBase::require(min_multiple_of_res, "(dccl.field).min must be an exact multiple of (dccl.field).resolution");
FieldCodecBase::require(max_multiple_of_res, "(dccl.field).max must be an exact multiple of (dccl.field).resolution");
}
else
{
auto res = resolution();
// this was previously allowed so we will only give a warning not throw an exception
if(!min_multiple_of_res)
dccl::dlog.is(dccl::logger::WARN, dccl::logger::GENERAL) && dccl::dlog << "Warning: (dccl.field).min should be an exact multiple of 10^(-(dccl.field).precision), i.e. " << res << ": " << this->this_field()->DebugString() << std::endl;
if(!max_multiple_of_res)
dccl::dlog.is(dccl::logger::WARN, dccl::logger::GENERAL) && dccl::dlog << "Warning: (dccl.field).max should be an exact multiple of 10^(-(dccl.field).precision), i.e. " << res << ": " << this->this_field()->DebugString() << std::endl;
}

// ensure that max and min are multiples of the resolution chosen
FieldCodecBase::require(std::abs(quantize(min(), resolution()) - min()) < min_max_eps, "(dccl.field).min must be an exact multiple of (dccl.field).resolution");
FieldCodecBase::require(std::abs(quantize(max(), resolution()) - max()) < min_max_eps, "(dccl.field).max must be an exact multiple of (dccl.field).resolution");

// ensure value fits into double
FieldCodecBase::require(std::log2(max() - min()) - std::log2(resolution()) <= std::numeric_limits<double>::digits,
"[(dccl.field).max-(dccl.field).min]/(dccl.field).resolution must fit in a double-precision floating point value. Please increase min, decrease max, or decrease precision.");
Expand Down
1 change: 1 addition & 0 deletions src/test/dccl_resolution/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ int main(int argc, char* argv[])
msg_in.set_u3(10.2);
msg_in.set_u4(5.6);
msg_in.set_u5(1.95);
msg_in.set_u6(25500);

std::string encoded;
codec.encode(&encoded, msg_in);
Expand Down
188 changes: 108 additions & 80 deletions src/test/dccl_resolution/test.proto
View file
Original file line number Diff line number Diff line change
@@ -1,106 +1,134 @@
@PROTOBUF_SYNTAX_VERSION@

import "dccl/option_extensions.proto";
package dccl.test;

message NumericMsg
{
option (dccl.msg).id = 10;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).precision = 12,
(dccl.field).in_head=true];

optional double b = 2 [(dccl.field).max = 18,
(dccl.field).min = -18,
(dccl.field).resolution = 0.0001];

// max is 2^64 rounded to 1e5
required uint64 u1 = 3 [(dccl.field).max = 18446744073709500000,
(dccl.field).min = 0,
(dccl.field).precision = -5];

// max is 2^64 rounded to 1e5
required uint64 u2 = 4 [(dccl.field).max = 18446744073709500000,
(dccl.field).min = 0,
(dccl.field).resolution = 100000];

// resolution != 10^N
required double u3 = 5 [(dccl.field).max = 15.5,
(dccl.field).min = 5.5,
(dccl.field).resolution = 0.5];

// resolution default
required double u4 = 6 [(dccl.field).max = 20.0,
(dccl.field).min = 0.0];

// weird resolution
required double u5 = 7 [(dccl.field).max = 3.6,
(dccl.field).min = 1.44,
(dccl.field).resolution = 0.12];


option (dccl.msg).id = 10;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [
(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).precision = 12,
(dccl.field).in_head = true
];

optional double b = 2 [
(dccl.field).max = 18,
(dccl.field).min = -18,
(dccl.field).resolution = 0.0001
];

// max is 2^64 rounded to 1e5
required uint64 u1 = 3 [
(dccl.field).max = 18446744073709500000,
(dccl.field).min = 0,
(dccl.field).precision = -5
];

// max is 2^64 rounded to 1e5
required uint64 u2 = 4 [
(dccl.field).max = 18446744073709500000,
(dccl.field).min = 0,
(dccl.field).resolution = 100000
];

// resolution != 10^N
required double u3 = 5 [
(dccl.field).max = 15.5,
(dccl.field).min = 5.5,
(dccl.field).resolution = 0.5
];

// resolution default
required double u4 = 6 [(dccl.field).max = 20.0, (dccl.field).min = 0.0];

// weird resolution
required double u5 = 7 [
(dccl.field).max = 3.6,
(dccl.field).min = 1.44,
(dccl.field).resolution = 0.12
];

// old precision field did not require min and max to be a multiple of resolution
required int32 u6 = 8 [
(dccl.field).min = 0,
(dccl.field).max = 25599,
(dccl.field).precision = -2
];
}

message NegativeResolutionNumericMsg // Invalid resolution
message NegativeResolutionNumericMsg // Invalid resolution
{
option (dccl.msg).id = 10;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [(dccl.field).min = -20,
(dccl.field).max = 20,
(dccl.field).resolution = -0.5];

optional int32 b = 2 [(dccl.field).min = -500000,
(dccl.field).max = 500000,
(dccl.field).precision = -3];
option (dccl.msg).id = 10;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [
(dccl.field).min = -20,
(dccl.field).max = 20,
(dccl.field).resolution = -0.5
];

optional int32 b = 2 [
(dccl.field).min = -500000,
(dccl.field).max = 500000,
(dccl.field).precision = -3
];
}


message BothResolutionAndPrecisionSetNumericMsg
{
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).precision = 1,
(dccl.field).resolution = 0.1];
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [
(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).precision = 1,
(dccl.field).resolution = 0.1
];
}

message TooBigNumericMsg
{
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).resolution = 1e-15];
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

optional double a = 1 [
(dccl.field).max = 180,
(dccl.field).min = -180,
(dccl.field).resolution = 1e-15
];
}

message MinNotMultipleOfResolution
{
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

required double a = 1 [(dccl.field).max = 3.6,
(dccl.field).min = 1.5,
(dccl.field).resolution = 0.2];
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

required double a = 1 [
(dccl.field).max = 3.6,
(dccl.field).min = 1.5,
(dccl.field).resolution = 0.2
];
}

message MaxNotMultipleOfResolution
{
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

required double a = 1 [(dccl.field).max = 3.5,
(dccl.field).min = 1.4,
(dccl.field).resolution = 0.2];
option (dccl.msg).id = 11;
option (dccl.msg).max_bytes = 32;
option (dccl.msg).codec_version = 4;

required double a = 1 [
(dccl.field).max = 3.5,
(dccl.field).min = 1.4,
(dccl.field).resolution = 0.2
];
}

0 comments on commit 3ce4696

Please sign in to comment.