From 4822d02f240cff2a2a35d952e9e64ddc19c39c54 Mon Sep 17 00:00:00 2001 From: Erica Fischer Date: Tue, 24 Sep 2024 23:55:43 -0700 Subject: [PATCH] Fix count accumulation in overzoom (#272) * Fix count accumulation in overzoom * Add test * Increment version and changelog --- CHANGELOG.md | 4 +++ Makefile | 6 ++++ attribute.cpp | 2 ++ clip.cpp | 43 +++++++++++++++++++++++------ tests/pbf/yearbuilt-accum.pbf.json | 7 +++++ tests/pbf/yearbuilt.pbf | Bin 0 -> 3535 bytes version.hpp | 2 +- 7 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 tests/pbf/yearbuilt-accum.pbf.json create mode 100644 tests/pbf/yearbuilt.pbf diff --git a/CHANGELOG.md b/CHANGELOG.md index e72daa75..7ed17c15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 2.62.4 + +* Fix accumulation of count and mean in overzoom + # 2.62.3 * Summary statistics with --accumulate-numeric-attributes make it from tiling through to binning diff --git a/Makefile b/Makefile index aa0564fc..74c94072 100644 --- a/Makefile +++ b/Makefile @@ -608,6 +608,12 @@ accumulate-test: test `./tippecanoe-decode -c tests/pbf/bins-0-0-0.pbf 0 0 0 | grep clustered:unrelated | wc -l` == 0 # the cluster sizes still add up to the 243 original features test `./tippecanoe-decode -c tests/pbf/bins-0-0-0.pbf 0 0 0 | sed 's/.*clustered:cluster_size": //' | awk '{sum += $$1} END {print sum}'` == 243 + # + # + # A tile where the counts and means were previously wrong: + ./tippecanoe-overzoom --accumulate-numeric-attributes=felt -m -o tests/pbf/yearbuilt-accum.pbf tests/pbf/yearbuilt.pbf 0/0/0 0/0/0 + ./tippecanoe-decode tests/pbf/yearbuilt-accum.pbf 0 0 0 > tests/pbf/yearbuilt-accum.pbf.json.check + cmp tests/pbf/yearbuilt-accum.pbf.json.check tests/pbf/yearbuilt-accum.pbf.json join-filter-test: tippecanoe tippecanoe-decode tile-join # Comes out different from the direct tippecanoe run because null attributes are lost diff --git a/attribute.cpp b/attribute.cpp index 5a06a4f4..0b00162a 100644 --- a/attribute.cpp +++ b/attribute.cpp @@ -159,9 +159,11 @@ void preserve_attribute(attribute_op const &op, std::string const &key, serial_v s.count = 2; attribute_accum_state.insert(std::pair(key, s)); + full_values[i].type = mvt_double; full_values[i].s = std::to_string(s.count); } else { // already present, incrementing state->second.count += 1; + full_values[i].type = mvt_double; full_values[i].s = std::to_string(state->second.count); } return; diff --git a/clip.cpp b/clip.cpp index c1bf81f1..b9343cb4 100644 --- a/clip.cpp +++ b/clip.cpp @@ -1186,13 +1186,18 @@ static void preserve_numeric(const std::string &key, const mvt_value &val, / // it is the wrong one. std::string outkey = key; + bool starting_from_accumulation; + if (starts_with(outkey, accumulate_numeric + ":")) { std::string prefix = accumulate_numeric + ":" + op.first + ":"; if (starts_with(outkey, prefix)) { outkey = outkey.substr(prefix.size()); + starting_from_accumulation = true; // from a subaccumulation } else { continue; // to next operation } + } else { + starting_from_accumulation = false; // from a plain value } // and then put it back on for the output field std::string prefixed = accumulate_numeric + ":" + op.first + ":" + outkey; @@ -1208,11 +1213,18 @@ static void preserve_numeric(const std::string &key, const mvt_value &val, / // not present at all, so copy our value to the prefixed output numeric_out_field.emplace(prefixed, full_keys.size()); full_keys.push_back(prefixed); + if (op.second == op_count) { - serial_val sv; - sv.type = mvt_double; - sv.s = "1"; - full_values.push_back(sv); + if (starting_from_accumulation) { + // copy our count + full_values.push_back(mvt_value_to_serial_val(val)); + } else { + // new count of 1 + serial_val sv; + sv.type = mvt_double; + sv.s = "1"; + full_values.push_back(sv); + } } else { full_values.push_back(mvt_value_to_serial_val(val)); } @@ -1220,20 +1232,35 @@ static void preserve_numeric(const std::string &key, const mvt_value &val, / // exists unprefixed, so copy it, and then accumulate on our value numeric_out_field.emplace(prefixed, full_keys.size()); full_keys.push_back(prefixed); + if (op.second == op_count) { serial_val sv; sv.type = mvt_double; - sv.s = "1"; + if (starting_from_accumulation) { + // sum our count onto the existing 1 + sv.s = std::to_string(1 + mvt_value_to_long_long(val)); + } else { + // sum our 1 onto the existing 1 + sv.s = "2"; + } full_values.push_back(sv); } else { full_values.push_back(full_values[out_attr->second]); + preserve_attribute(op.second, prefixed, mvt_value_to_serial_val(val), full_keys, full_values, attribute_accum_state); } - - preserve_attribute(op.second, prefixed, mvt_value_to_serial_val(val), full_keys, full_values, attribute_accum_state); } } else { // exists, so accumulate on our value - preserve_attribute(op.second, prefixed, mvt_value_to_serial_val(val), full_keys, full_values, attribute_accum_state); + if (op.second == op_count) { + if (starting_from_accumulation) { + // sum our count onto the existing count + full_values[prefixed_attr->second].s = std::to_string(atoll(full_values[prefixed_attr->second].s.c_str()) + mvt_value_to_long_long(val)); + } else { + full_values[prefixed_attr->second].s = std::to_string(atoll(full_values[prefixed_attr->second].s.c_str()) + 1); + } + } else { + preserve_attribute(op.second, prefixed, mvt_value_to_serial_val(val), full_keys, full_values, attribute_accum_state); + } } } } diff --git a/tests/pbf/yearbuilt-accum.pbf.json b/tests/pbf/yearbuilt-accum.pbf.json new file mode 100644 index 00000000..183bac43 --- /dev/null +++ b/tests/pbf/yearbuilt-accum.pbf.json @@ -0,0 +1,7 @@ +{ "type": "FeatureCollection", "properties": { "zoom": 0, "x": 0, "y": 0 }, "features": [ +{ "type": "FeatureCollection", "properties": { "layer": "parsed", "version": 2, "extent": 4096 }, "features": [ +{ "type": "Feature", "id": 510, "properties": { "blklot": "0858005", "block_num": "0858", "from_st": "222", "landuse": "RESIDENT", "lot_num": "005", "mapblklot": "0858005", "st_type": "ST", "street": "WALLER", "to_st": "222", "bldgsqft": 1765, "cie": 0, "med": 0, "mips": 0, "objectid": 11031, "pdr": 0, "resunits": 1, "retail": 0, "shape_area": 2279.71121678, "shape_leng": 212.495847301, "total_uses": 0, "visitor": 0, "yrbuilt": 1900, "felt:count:bldgsqft": 1134, "felt:max:bldgsqft": 517232, "felt:min:bldgsqft": 0, "felt:sum:bldgsqft": 4857699, "felt:count:cie": 1134, "felt:max:cie": 20780, "felt:min:cie": 0, "felt:sum:cie": 159842, "felt:count:med": 1134, "felt:max:med": 13747, "felt:min:med": 0, "felt:sum:med": 65490, "felt:count:mips": 1134, "felt:max:mips": 35150, "felt:min:mips": 0, "felt:sum:mips": 493890, "felt:count:objectid": 1134, "felt:max:objectid": 155370, "felt:min:objectid": 10714, "felt:sum:objectid": 38161243, "felt:count:pdr": 1134, "felt:max:pdr": 18000, "felt:min:pdr": 0, "felt:sum:pdr": 259718, "felt:count:resunits": 1134, "felt:max:resunits": 94, "felt:min:resunits": 0, "felt:sum:resunits": 4269, "felt:count:retail": 1134, "felt:max:retail": 12898, "felt:min:retail": 0, "felt:sum:retail": 425966, "felt:count:shape_area": 1134, "felt:max:shape_area": 187422.348941, "felt:min:shape_area": 824.484455232, "felt:sum:shape_area": 3770890.8948438765, "felt:count:shape_leng": 1134, "felt:max:shape_leng": 2741.43546213, "felt:min:shape_leng": 121.331962321, "felt:sum:shape_leng": 306327.034212475, "felt:count:total_uses": 1134, "felt:max:total_uses": 63028, "felt:min:total_uses": 0, "felt:sum:total_uses": 1425446, "felt:count:visitor": 1134, "felt:max:visitor": 15000, "felt:min:visitor": 0, "felt:sum:visitor": 20540, "felt:count:yrbuilt": 1134, "felt:max:yrbuilt": 2014, "felt:min:yrbuilt": 1878, "felt:sum:yrbuilt": 2165833, "felt:cluster_size": 113, "felt:mean:bldgsqft": 4283.685185185185, "felt:mean:cie": 140.9541446208113, "felt:mean:med": 57.75132275132275, "felt:mean:mips": 435.5291005291005, "felt:mean:objectid": 33651.8897707231, "felt:mean:pdr": 229.02821869488538, "felt:mean:resunits": 3.7645502645502648, "felt:mean:retail": 375.63139329805997, "felt:mean:shape_area": 3325.3006127371047, "felt:mean:shape_leng": 270.1296597993607, "felt:mean:total_uses": 1257.0070546737214, "felt:mean:visitor": 18.112874779541447, "felt:mean:yrbuilt": 1909.905643738977 }, "geometry": { "type": "Point", "coordinates": [ -122.343750, 37.718590 ] } } +, +{ "type": "Feature", "id": 514, "properties": { "blklot": "0858003", "block_num": "0858", "from_st": "210", "landuse": "MIXRES", "lot_num": "003", "mapblklot": "0858003", "st_type": "ST", "street": "WALLER", "to_st": "210", "bldgsqft": 3050, "cie": 0, "med": 0, "mips": 1678, "objectid": 10950, "pdr": 0, "resunits": 2, "retail": 0, "shape_area": 1885.04187192, "shape_leng": 201.483365997, "total_uses": 1678, "visitor": 0, "yrbuilt": 1900, "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ -122.343750, 37.718590 ] } } +] } +] } diff --git a/tests/pbf/yearbuilt.pbf b/tests/pbf/yearbuilt.pbf new file mode 100644 index 0000000000000000000000000000000000000000..892ec1723857638b52d954ff0a44ec8e7db3ab79 GIT binary patch literal 3535 zcmV;=4KVT_iwFP!000006OER6V9eJR$8&F%nS_{#eQYs=Btj%1W;Cg#sNJ(vRUZ*C zbab-FOkRiTYwSg5Y_UZx5n^o-TM$cPeMHcbA`;pLFZQaUmU`!VMkXWvd4JsV``&Z! zJ-^@Qo^$Vv`f_WF^zulINiyk22c#&~%2CGAV@>17nAIMmjGr45%xce3#)R0vC*Z|i<^*$$F`8a&QY-#tFd57VNovJ}q*2KRqgh>JjNWMO5Sx%3Z|>lLsm{*0 z7|S~@LwseIDLJmfRm~|H=&L#cIrS(;MGwo-b6$zfs95RLqm>mstVB=cyb?R;U9r@u zi?tP9UW~3Yzhmbs_$#|8N+lN}Ib6;Z$X*47Q;%X)^spQ~=kIW&yy815yC_B_7Xnps zl}F>UQ8~qOc5oyohdmBEYB@!5D!LkuNbfo++B;4}baK8MmCLtsRA;9ns*|%KDm$H{ zIu(3H)OVc}^&KaoIyrrvqLv>WA4kYa9*Xz22f@lc&XLN`UPYuz9*Xp~2a(D>Z*pr) zHktKF(I&%0y*k)zNKDkn#>6M+?YWDOPE0Vwn@!Pi$wnG88Zc8oK3N|ht5*m9KeNXe zl1yfeo0l9kSmW;H+NMpY{Sjt=gxeqO?2ib3gui)c`O&tm{bBF5~@;r0iw3vFw6 zw$<^YoqenwkA>Oa7Z%!vA7NqkN4Wjbo*&_%8V@hGHrjSt`!hnPQF?jw?KyN{w?R-J zcka`t+dz${mjZL}%?{blE+cf{a>%d>8D1eH95UQVY8{e59kRkpz+PU;E+2i+qjSH` z{h;ZqdUoyBZ?ML*o{PGF@7-Tc?F89f|EhgT2Bb_qw_f$tqE;;8 z35_xb{*gB;8nSVxbM@UKCPQk59Q^E9X$fTD0sjXnozo#(E_;5udMEYhxZ<9r&${y`+Eo7t2Kb*_FwAx?7*g)nEQ_tJ+t~&;cpnYuYcL_r}>aW z{CwgEmdu9y#8Q%$bF&8Ke!3&zMe*Em@Wy!5YrpZk8<56_32jTi;0dNXJ99Lx%~Icb5w>JA2ZwHU07+_YL{y={OhO>O&(B4wZ^``FN!Qo`XaVF)a01a zh2)I&-ehN|OV~Utn}cO@hQf>`=Ny{?HXjqL8JJ-6h6y1vw?3J98XB4RN|2myAvr$- z;|m6mS?Egrg+-nuQ~QuiU5uBdE=82Z4M;9dZ9sC#5Ryw4U~EZ|D>;@LV3Mi7Y&Ce6 zWr8hj3ES81WS3`Q-ghu7@RAjUKwKF@a^?JbWLNE0l4CU{tagK`22+oAYR&%F_|}Z( zWY+q_#E@C{3CuW{1e&yN)-RYu_2llC`T8baydf76H${=$vIc|Sw;=h$MR>PPAhT@% z^|vhp#`CTyM;bm|0@i$__l$~OerE0& zd|m-Qw*ZR@v^1+=0bYF;{j+{#&fz1@eT$jr;rx+JEtrvHE;ORyi;EGouo}t2E%@BR zt$5YXn0g7ByA(mrOG~h~m)2qKWqjJ@Ve}Q3_di7JE2zaQgJ{B)$LJTeBw4fwD~i_P z3$J4AY8qrQLKY9A#l;!7Fu4RTD(OWNu5I>&%?1Pg!nyffXUFv|SoiB_lE2=-ir>Ov z29YU6+|o@Lzkx5if&Pt5Ou7kq^BM~DRvEDVo5nT=A?p= zXV~#)8xi#{%zEC8%!^2JzStpYuU}-BVeA#gUhB#IdJ8`B^5xA1UNG*DVREnYiB6-(6#-j-@fstA4PrezEyO*DLPp?XNxXY z@qtZtwRE$|?pE=kRrXjcd#1}?R?&M>pZikzx2wk8^Q2tr;d>5Lxx(d{5KUxVqN+_) z9*QPRl_K9znN?$%sy0*26}4@m&LdDg5NeyK|AQc^QrXBR8ef-0wOeRxqUlUf?HA2$ zqQyZOuu=w=h#;G2ITuvBL~EM}nFXq8GR!8zw}UE2MA$_8-r59RVh5XeZ)jwnDBb%u z(eWCnewJNq;)6YcsP>5NHu2#i*<+pTxlH!5iQZo-XUTG;^FYcIF24xTh>?5%&O5IZ^jT-6f)4E@$dEn6^k{>LoJuk~6hl5YrlgO#LPTcF4f> z5}CRMrjwkh>0m05$kgwGnC^0>+Gxk?x+7C#BU8sEG5sVfWokNSYNtS^{;izm zR+(v?5Dl5Bx_dnjp=d}M&z;sBsOqRl*nW|ErycGVNr&BCP$i;V0W)gGmH3qv zpgo+TsQ@~~b$1&;#hfC&wxg~mQWO{2FKQT4bR9r>Z>Q)RuDY$9qRB!z-;ts-AUY^q z&P%!j%ZU^U4HI3gOjOJ1&fA6*-D9GkK(v{Q&O$&I@T>yQ3NE=E8SqL3-esOyD7kyg zGYvdjxa4l4LUJDHDCIobCY12haiGGEWLW z2C70p?g~E!x(c941oWP&7Xwvcpj5uTFCal#e1`a{`%~Zy*%L%3=r9)?pP>K9fD9Q} zz?b(N;C#oI_sohDGy^z0`2!Xf}=Fwheb@K^+v z@s)5KCn%S%gc$%j$Uv`gg0}Im(fiu|x={f7A~HOxHGo#5;x4@n^i%-o7yd1psQkw9 zThyI+v@VCFsLy{HeCgJ9Rn#Y(wZiMK@+&S>^-n%OzBSeg2I@7T_gDCto>HFY{{eu+ JBRE_a004?G)t&$V literal 0 HcmV?d00001 diff --git a/version.hpp b/version.hpp index ac10795b..90c6eb88 100644 --- a/version.hpp +++ b/version.hpp @@ -1,6 +1,6 @@ #ifndef VERSION_HPP #define VERSION_HPP -#define VERSION "v2.62.3" +#define VERSION "v2.62.4" #endif