search

elastic / ml-cpp

Machine learning C++ code
Other
149 stars 62 forks source link

[ML] Remove CStringStore #2652

Closed jan-elastic closed 3 months ago

jan-elastic commented 4 months ago

It's probably most convenient to review this commit by commit. All commits are self-contained and the tests pass after each commit.

jan-elastic commented 4 months ago

@tveasey What end-to-end testing do you think is good to do to verify these changes?

tveasey commented 4 months ago

@tveasey What end-to-end testing do you think is good to do to verify these changes?

Let's first see if any results change in the overnights (this should be a no op in terms of behaviour). This should also give us visibility of any performance degradations.

Once we are happy with that, we can try a local test with one of the overnights with many distinct partitions + over field values. I would test:

  1. Memory is controlled effectively (also check the running process memory monitored independently) for different settings of limit
  2. We use broadly similar memory (of course it doesn't matter if we actually use less)
tveasey commented 4 months ago

Only very minor things

We also look to have a failure in CHierarchicalResultsTest: I guess there is some subtle change in behaviour.

wwang500 commented 4 months ago

buildkite run_qa_tests

wwang500 commented 4 months ago

buildkite run_qa_tests