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

fix: treating unary minus in calcString #784

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

bangonicdd2
Copy link
Contributor

PR Checklist

  • Have you checked if it works normally in all models? Ignore this if it doesn't use models.
  • Have you checked if it works normally in all web, local, and node hosted versions? If it doesn't, have you blocked it in those versions?
  • Have you added type definitions?

Description

It has been reported that calcString incorrectly calculates expressions when a negative number appears on the right-hand side. This pull request addresses this issue.

@bangonicdd2
Copy link
Contributor Author

I also found a report that the '!=' symbol exists in the docs but doesn't actually work, so I added another commit. Thanks for your time.

@kwaroran
Copy link
Owner

Wouldn't these break the existing bots?

@bangonicdd2
Copy link
Contributor Author

bangonicdd2 commented Mar 13, 2025

I've checked that all the expressions in this post: https://arca.live/b/characterai/130904511
which raised concerns about unary operators, are working correctly.
Or might you have any specific part that you're worried about?
Anyway, I'll test some additional cases and report the results in a comment later

@bangonicdd2
Copy link
Contributor Author

bangonicdd2 commented Mar 14, 2025

Following is additional cases I've tested:

expression expected main branch pr branch
{{? 5+3}} 8 8 8
{{? 5-3}} 2 2 2
{{? 5+-3}} 2 2 2
{{? 5-(-3)}} 8 2 8
{{? -5+3}} -2 -2 -2
{{? 10-5*-2}} 20 8 20
{{? 10/(-2)}} -5 Infinity -5
{{? 2^(-3)}} 0.125 -2 0.125
{{? (-5)*(-3)}} 15 -3 15
{{? 5!=3}} 1 0 1
{{? 5!=5}} 0 0 0
{{? 2+3!=1+4}} 0 0 0
{{? 5+3!=9}} 1 0 1
{{? (5!=3)&&(2==2)}} 1 0 1
{{? 5*4}} 20 20 20
{{? 20/4}} 5 5 5
{{? 10%3}} 1 1 1
{{? 2^3}} 8 8 8
{{? !0}} 1 1 1
{{? !1}} 0 0 0
{{? 5>3}} 1 1 1
{{? 5>=5}} 1 1 1
{{? 5<8}} 1 1 1
{{? 5<=5}} 1 1 1
{{? 5==5}} 1 1 1
{{? 5==3}} 0 0 0
{{? 5||0}} true 5 5
{{? 5&&0}} 0 0 0
{{? (5+3)*2}} 16 16 16
{{? (10-3)+(2^3)*2)}} 23 23 23

@sirius422
Copy link

Indeed, I also noticed the abnormal behavior of != in numerical calculations today.

expression (calcString) result expression (basicMatcher) result
{{? 2 != 0}} 0 not_equal(2,0) 1
{{? 2 != 1}} 1 not_equal(2,1) 1
{{? 2 != 2}} 0 not_equal(2,2) 0
{{? 2 != 3}} 0 not_equal(2,3) 1
{{? 2 != 4}} 0 not_equal(2,4) 1

For example, {{? 2 != 3}}, this occurs because != is not defined in function toRPN, so when converting it from Infix notation to RPN, it's treated as 2 !0 = 3, and 2 was also discarded in the stack calculation.
{{? 2 != 3}} --parsed inside toRPN--> [ "2", "!", "0", "=", "3"] --toRPN returned--> "2 0 ! 3 = "

I was about to make a pull request when I noticed your work. I'm glad to see that commit 681846c resolved this issue.

@kwaroran kwaroran merged commit 15eb4de into kwaroran:main Mar 17, 2025
@bangonicdd2 bangonicdd2 deleted the calcString branch March 18, 2025 07:13
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.

3 participants