velox
velox copied to clipboard
Add Presto inverse_cauchy_cdf function
Resolves https://github.com/facebookincubator/velox/issues/5301 This is a followup for https://github.com/facebookincubator/velox/pull/5341
Deploy Preview for meta-velox canceled.
| Name | Link |
|---|---|
| Latest commit | bcdf5293d9901a5f018e568e1244eef461677966 |
| Latest deploy log | https://app.netlify.com/sites/meta-velox/deploys/668ccc8de277130008642acf |
@wills-feng : Please add a fuzzer run output.
@aditi-pandit I rewrote the function and tests. Could you please review, thanks?
I20240422 12:18:39.055817 28617570 ExpressionFuzzerVerifier.cpp:367] ==============================> Done with iteration 174940
I20240422 12:18:39.055881 28617570 ExpressionFuzzerVerifier.cpp:164] ==============================> Top 1 by number of rows processed
I20240422 12:18:39.055895 28617570 ExpressionFuzzerVerifier.cpp:166] Format: functionName numTimesSelected proportionOfTimesSelected numProcessedRows
I20240422 12:18:39.055910 28617570 ExpressionFuzzerVerifier.cpp:170] inverse_cauchy_cdf 3969852 100.00% 4032571
I20240422 12:18:39.055929 28617570 ExpressionFuzzerVerifier.cpp:176] ==============================> Bottom 1 by number of rows processed
I20240422 12:18:39.055945 28617570 ExpressionFuzzerVerifier.cpp:178] Format: functionName numTimesSelected proportionOfTimesSelected numProcessedRows
I20240422 12:18:39.055961 28617570 ExpressionFuzzerVerifier.cpp:183] inverse_cauchy_cdf 3969852 100.00% 4032571
I20240422 12:18:39.055975 28617570 ExpressionFuzzerVerifier.cpp:196] ==============================> All stats sorted by number of times the function was chosen
I20240422 12:18:39.055992 28617570 ExpressionFuzzerVerifier.cpp:198] Format: functionName numTimesSelected proportionOfTimesSelected numProcessedRows
I20240422 12:18:39.056008 28617570 ExpressionFuzzerVerifier.cpp:202] inverse_cauchy_cdf 3969852 100.00% 4032571
E20240422 12:18:39.056025 28617570 ExpressionFuzzerVerifier.cpp:373] Total iterations: 174941
E20240422 12:18:39.056042 28617570 ExpressionFuzzerVerifier.cpp:374] Total failed: 11613
[==========] Running 0 tests from 0 test suites.
[==========] 0 tests from 0 test suites ran. (0 ms total)
[ PASSED ] 0 tests.
@wills-feng : Code looks good.
Though your release build has an error
velox/connectors/hive/storage_adapters/hdfs/tests/CMakeFiles/velox_hdfs_file_test.dir/HdfsFileSystemTest.cpp.o -MF velox/connectors/hive/storage_adapters/hdfs/tests/CMakeFiles/velox_hdfs_file_test.dir/HdfsFileSystemTest.cpp.o.d -o velox/connectors/hive/storage_adapters/hdfs/tests/CMakeFiles/velox_hdfs_file_test.dir/HdfsFileSystemTest.cpp.o -c /__w/velox/velox/velox/connectors/hive/storage_adapters/hdfs/tests/HdfsFileSystemTest.cpp
/__w/velox/velox/velox/connectors/hive/storage_adapters/hdfs/tests/HdfsFileSystemTest.cpp: In static member function 'static void HdfsFileSystemTest::SetUpTestSuite()':
/__w/velox/velox/velox/connectors/hive/storage_adapters/hdfs/tests/HdfsFileSystemTest.cpp:49:38: error: 'using element_type = class facebook::velox::exec::test::TempFilePath' {aka 'class facebook::velox::exec::test::TempFilePath'} has no member named 'path'; did you mean 'const string facebook::velox::exec::test::TempFilePath::path_'? (accessible via 'const string& facebook::velox::exec::test::TempFilePath::getPath() const')
49 | miniCluster->addFile(tempFile->path, destinationPath);
| ^~~~
| getPath()
[1473/1978] Building CXX object velox/connectors/hive/storage_adapters/hdfs/CMakeFiles/velox_hdfs.dir/HdfsWriteFile.cpp.o
[1474/1978] Building CXX object velox/connectors/hive/storage_adapters/hdfs/CMakeFiles/velox_hdfs.dir/RegisterHdfsFileSystem.cpp.o
[1475/1978] Building CXX object velox/connectors/hive/iceberg/tests/CMakeFiles/velox_hive_iceberg_test.dir/IcebergReadTest.cpp.o
[1476/1978] Building CXX object velox/connectors/hive/storage_adapters/s3fs/tests/CMakeFiles/velox_s3registration_test.dir/S3FileSystemRegistrationTest.cpp.o
[1477/1978] Building CXX object velox/connectors/hive/storage_adapters/hdfs/tests/CMakeFiles/velox_hdfs_file_test.dir/InsertIntoHdfsTest.cpp.o
[1478/1978] Building CXX object velox/connectors/hive/storage_adapters/s3fs/tests/CMakeFiles/velox_s3multiendpoints_test.dir/S3MultipleEndpointsTest.cpp.o
[1479/1978] Building CXX object velox/connectors/hive/storage_adapters/s3fs/tests/CMakeFiles/velox_s3insert_test.dir/S3InsertTest.cpp.o
ninja: build stopped: subcommand failed.
make[1]: *** [Makefile:103: build] Error 1
make[1]: Leaving directory '/__w/velox/velox'
make: *** [Makefile:110: release] Error 2
Maybe you need to rebase ?
Please let me know once you get all clean checks.
Thanks @wills-feng.
@Yuhta @kagamiori : Please can you help with review and merge.
These functions are from old PRs which were held back because of the boost issues found in the fuzzer runs. These are being moved ahead since boost is fixed and updated now.
There are 3 probability functions PRs are waiting for merge. I have rebased https://github.com/facebookincubator/velox/pull/9492, once it gets merged, I will rebase this one.
@wills-feng I just merged #9492. Could you go ahead and rebase this one?
@pedroerp Thank you. Rebased this one. CI looks good.
@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pedroerp merged this pull request in facebookincubator/velox@ba8d27da00d8eca253a64585ac86262783dd2803.
Conbench analyzed the 1 benchmark run on commit ba8d27da.
There were no benchmark performance regressions. 🎉
The full Conbench report has more details.