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

Improved Log inference #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rowanG077
Copy link
Member

@rowanG077 rowanG077 commented Feb 9, 2022

Normalise Log base (a * b * ... * z) to Log base a + Log base b + ... + Log base z
Normalise Log b (x^y) to y * Log b x

@rowanG077 rowanG077 force-pushed the master branch 2 times, most recently from e6640a1 to 2b7bd88 Compare February 9, 2022 17:13
@rowanG077
Copy link
Member Author

rowanG077 commented Feb 9, 2022

While working on it I'm not actually sure this is legal for naturals in the way the solvers function.

For example Log 10 (50 * 2) prior to this PR will reduce to Log 10 100 -> 2. But depending on the way the plugins fire after this PR it might reduce like this: Log 10 (50 * 2) -> Log 10 50 + Log 10 2 and now it will report this as unsolvable as both the logs will not cleanly reduce to a Natural.

I feel like similar issues exist for the other rule but I don't have an example handy.

I guess doing the rewrites the other way should be fine.

@rowanG077
Copy link
Member Author

rowanG077 commented Feb 10, 2022

Ready for review

I choose not to normalise x * log b a to log b a^x because I feel like dealing with exponents sucks. And the rewrite the other way around is unsound.

note that I used the withSOPNormalize from this closed PR: #36

@alex-mckenna
Copy link

because I feel like dealing with exponents sucks

I can already see @christiaanb reading this and going "yep" 👀

Normalise `Log base a + Log base b + ... + Log base z` + `Log base (a * b * ... * z)`
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