-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Constant folding for cast from number to string #931
Labels
good first issue
Non-urgent, simple task with limited scope, suitable for getting started as a contributor.
Comments
pdamme
added a commit
that referenced
this issue
Dec 12, 2024
- Cleaned up the directory "test/api/cli/io/". - Problems with previous state: - The test cases mixed some things that should be tested independently, e.g., reading a frame and reading from a file whose path is given as the parameter of a DaphneDSL UDF. - The directory was cluttered with test data/metadata files (reference files and git-ignored generated files). - Inconsistent naming of test files. - Consequently, adding new test cases into the existing structure was not straightforward. - Changes by this commit: - Rewrote test cases that read matrices/frames with and without string data. - Removed the old .csv and .csv.meta files and replaced them by new (sometimes simpler) files (in subdirectory "ref/") that still check the same interesting cases (and more). - Removed the old "testRead*.{daphne,txt}" files. - The DaphneDSL files were replaced by the new ones in subdirectory "read/". - The reference txt files are omitted, since comparisons are not based on DaphneDSL print() anymore (since its output can be ambiguous for string data). - Replaced "ReadTest.cpp" and "WriteTest.cpp" by a single "ReadWriteTest.cpp" to have all test cases at a glance. - Removed the dynamic path test, replaced it by several new ones. - So far, this test case was commented out. - Now, variants that just concatenate strings are tested, because they already work. - However, variants concatenating strings and numbers don't work yet and remain commented (see #931). - Removed unused files "readSparse.daphne" and "readSparse.txt": They used to be part of a test case for reading sparse data into a CSRMatrix, but that test case was commented out and the file "readSparse.coo" referenced in it didn't even exist. Thus, we don't test reading sparse data into CSRMatrix now, but we didn't do that before this clean-up, either. - Created new DaphneDSL scripts that test writing matrices/frames (subdirectory "write/"). - After this clean-up, the tests still cover the same interesting cases (though sometimes with different examples). - Fixed a bug in CSV reader for string data that was revealed by rewriting the read test cases. - Escaped double-quote characters in quoted CSV values were not unescaped, i.e., they remained double double-quotes "" in memory, although they should become single double-quotes ". - Consistently, a bug in the unit test cases was fixed. - Added some required cast-kernel specializations. - A specialization of CastObj from Frame to DenseMatrix<std::string>. - A specialization of CastSca from std::string to std::string (required for avoiding template ambiguities). - Thanks to @saminbassiri for this cast-kernel code.
Search for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
good first issue
Non-urgent, simple task with limited scope, suitable for getting started as a contributor.
The DAPHNE compiler can evaluate various operations on constant scalars at compile-time. The respective constant folding implementations can be found in
src/ir/daphneir/Fold.cpp
. While casts of scalars between various value types can already be folded (seemlir::OpFoldResult mlir::daphne::CastOp::fold(FoldAdaptor adaptor)
), casts from numbers to strings are not folded yet. Folding such casts would be useful as it would allow DaphneDSL users to do things like:Which is currently not possible as the file name cannot be resolved at compile-time.
The text was updated successfully, but these errors were encountered: