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 for +-rule-down #15

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

Conversation

hacdevilliers
Copy link

Argument presented in the source code was
;; note: We can't assert that X and Y are integers when Z is an integer since
;; Z may be an integer when X and Y are Gaussian integers. But we can
;; make such an assertion if either X or Y is real. If the Screamer
;; type system could distinguish Gaussian integers from other complex
;; numbers we could make such an assertion whenever either X or Y was
;; not a Gaussian integer.
The argument above must surely be incorrect. If Z is an integer, and Y is a real
say 2.5 then X will surely not be an integer. The argument does work if eithe
r X or Y are integers. So changed things so that Z being an integer and X OR Y being an INTEGER means that X AND Y are both integers. (Note the assertion is only for X, but this is correct given that the assertion for Y happens in another call to this rule.

Argument presented in the source code was
  ;; note: We can't assert that X and Y are integers when Z is an integer since
  ;;       Z may be an integer when X and Y are Gaussian integers. But we can
  ;;       make such an assertion if either X or Y is real. If the Screamer
  ;;       type system could distinguish Gaussian integers from other complex
  ;;       numbers we could make such an assertion whenever either X or Y was
  ;;       not a Gaussian integer.
The argument above must surely be incorrect. If Z is an integer, and Y is a real
 say 2.5 then X will surely not be an integer. The argument *does* work if eithe
r X or Y are integers. So changed things so that Z being an integer and X OR Y being an INTEGER means that X AND Y are both integers. (Note the assertion is only for X, but this is correct given that the assertion for Y happens in another call to this rule.
@hacdevilliers
Copy link
Author

This is just the text of the original issue that I opened (sorry about the duplication, haven't used github for this sort of thing before)

function -v erroneously restricts arguments to integers in certain cases

I'm a bit too busy to delve into this issue more deeply at the moment (maybe I'll poke at the screamer sources later, haven't really had a look at them before), but

(-v (a-real-betweenv 1.1 1.2))

emits an error. (This form distils the issue I had in much more complicated code) Mysteriously, the real variable created by a-real-between-v has been restricted to be an integer, causing the exception later on. Further investigation reveals

(-v 0 (a-real-betweenv 1.1 1.2))

has the same issue, but the following works correctly

(-v 0.0 (a-real-betweenv 1.1 1.2))

This can be used as a workaround for anyone similarly affected.

In the case of +v, there is seemingly no such problem.

(+v (a-real-betweenv 1.1 1.2))
(+v 0 (a-real-betweenv 1.1 1.2))
(+v 0.0 (a-real-betweenv 1.1 1.2))

all work correctly. Perhaps this means a fix may be relatively simple?

(Note: I'm using the quicklisp screamer-20111105-git distro in case this matters)

Copy link
Owner

@nikodemus nikodemus left a comment

Choose a reason for hiding this comment

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

A test case that demonstrates the problem would appreciated. (In tests.lisp)

swapneils added a commit to swapneils/screamer that referenced this pull request Feb 10, 2024
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.

None yet

2 participants