Skip to content
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

to_int/from_int optimization #116

Merged
merged 4 commits into from
Feb 6, 2024
Merged

to_int/from_int optimization #116

merged 4 commits into from
Feb 6, 2024

Conversation

jurajsic
Copy link
Member

@jurajsic jurajsic commented Jan 29, 2024

I added different handling for the part that contain some non-digit during the creation of LIA formula for to_int/from_int so that only the part of the language with words containing only digits must be finite.

@jurajsic jurajsic changed the title To from int 2 to_int/from_int optimization Jan 31, 2024
@jurajsic
Copy link
Member Author

jurajsic commented Feb 1, 2024

Impact on stringfuzz (nearly no impact on other benchmarks):

Benchmark stringfuzz/to120.csv
# of formulae: 11618
--------------------------------------------------
tool                           ✅    ❌    avg    med      time    sat    unsat    unknown    TO    MO+ERR    other
--------------------------  -----  ----  -----  -----  --------  -----  -------  ---------  ----  --------  -------
z3-noodler-7421e81-2cddb2f  11347   271   0.03   0.01    291.77   6165     5182        266     5         0        0
cvc5-1.1.1                  10522  1096   2.65   0.00  27893.23   5756     4766          0  1095         1        0
z3-4.12.5                   11072   546   4.09   0.01  45283.01   5986     5086          0   421       125        0
z3-noodler-147a0e0-fb6e2b1  10574  1044   0.02   0.01    264.03   6087     4487       1044     0         0        0
--------------------------------------------------

@jurajsic jurajsic marked this pull request as ready for review February 1, 2024 19:05
@jurajsic jurajsic requested review from vhavlena and Adda0 February 1, 2024 19:05
Copy link
Collaborator

@Adda0 Adda0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot see anything wrong. But I only skimmed the code. I am too ill to do a proper review.

if (word_of_subst_var.size() == 1) { // |w_i| = 1
mata::Symbol code_point = word_of_subst_var[0]; // this must be a code point of a digit, as w_i can only contain digits
formula_for_case.succ.emplace_back(LenFormulaType::EQ, std::vector<LenNode>{ code_version_of(subst_var), code_point });
} // "else" part is not needed, that should be solved by setting "|s_i| = |w_i|" and by using the forula from function get_formula_for_code_subst_vars()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

Suggested change
} // "else" part is not needed, that should be solved by setting "|s_i| = |w_i|" and by using the forula from function get_formula_for_code_subst_vars()
} // "else" part is not needed, that should be solved by setting "|s_i| = |w_i|" and by using the formula from function get_formula_for_code_subst_vars()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the next PR

@jurajsic
Copy link
Member Author

jurajsic commented Feb 6, 2024

Ok, I will merge it for now, @vhavlena can look at it after he comes back (if he wants).

@jurajsic jurajsic merged commit ef60372 into devel Feb 6, 2024
@jurajsic jurajsic deleted the to-from-int-2 branch February 6, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants