[ML] add geo point formatting string for actual|typical #809
Conversation
| std::ostringstream sstream; | ||
| std::string partitionFieldName("tfn"); | ||
| std::string partitionFieldValue(""); | ||
| std::string overFieldName("pfn"); |
There was a problem hiding this comment.
Maybe change these string values to ofv, ofn for consistency?
| BOOST_TEST_REQUIRE(object.IsObject()); | ||
|
|
||
| BOOST_TEST_REQUIRE(object.HasMember("typical")); | ||
| //BOOST_REQUIRE_EQUAL(2, object["typical"].GetArray().Size()); |
There was a problem hiding this comment.
Hmmm.. I'm always in 2 minds about commented out code, maybe delete it if it's not needed?
There was a problem hiding this comment.
yeah, I cannot get the tests to do what I want the to do :(. So, very WIP there.
lib/api/CJsonOutputWriter.cc
Outdated
| // We don't want scientific notation and geo points only have precision up to 12 digits | ||
| ptStr << std::fixed; | ||
| ptStr << std::setprecision(12); | ||
| ptStr << results.s_BaselineMean[0] << "," << results.s_BaselineMean[1]; |
There was a problem hiding this comment.
I'd be inclined to factor this out as a lambda (or function), i.e.
auto geoPointToString = [](const auto& point) -> std::string {
std::ostringstream result;
return (result << std::fixed << std::setprecision(12) << point[0] << "," << point[1]).str();
};
|
Looks good to go @benwtrent |
docs/CHANGELOG.asciidoc
Outdated
| @@ -37,6 +37,7 @@ | |||
| * Reduce the peak memory used by boosted tree training and fix an overcounting bug | |||
| estimating maximum memory usage. (See {ml-pull}781[#781].) | |||
| * Stratified fractional cross validation for regression. (See {ml-pull}784[#784].) | |||
| * Added `geo_point` supported output for `lat_long` function records. (See {ml-pull}809[#809].) | |||
There was a problem hiding this comment.
Because of the title of elastic/elasticsearch#47050, as things stand this release note will appear in addition to "Add new geo_results.(actual|typical) fields for lat_lon results". As a user I would wonder whether these two things were the same or different enhancements, so it's probably best to have just one of them.
Since this changelog is more flexible and can reference two PRs, it's probably best to mark elastic/elasticsearch#47050 as a non-issue so that it doesn't get auto-generated into the release notes, but then mention it in this one too, using (See {ml-pull}809[#809], {pull}47050[#47050].).
…_long` results (#47050) [ML] Add new geo_results.(actual_point|typical_point) fields for `lat_long` results (#47050) Related PR: elastic/ml-cpp#809
…_long` results (elastic#47050) [ML] Add new geo_results.(actual_point|typical_point) fields for `lat_long` results (elastic#47050) Related PR: elastic/ml-cpp#809
…r `lat_long` results (#47050) (#48958) * [ML] Add new geo_results.(actual_point|typical_point) fields for `lat_long` results (#47050) [ML] Add new geo_results.(actual_point|typical_point) fields for `lat_long` results (#47050) Related PR: elastic/ml-cpp#809 * adjusting bwc version
Adds new geo_point formatted fields for Java side consumption. These are only written if we have actual|typical values that have length of 2, and are using a lat_long function. This is my first PR against this repo, let me know if I missed something. related elastic/elasticsearch#47050
Adds new geo_point formatted fields for Java side consumption. These are only written if we have actual|typical values that have length of 2, and are using a
lat_longfunction.This is my first PR against this repo, let me know if I missed something.
Blocked by elastic/elasticsearch#47050