Opened 4 years ago

Closed 4 years ago

#18695 closed defect (fixed)

propagate the variable domain to the assumptions database

Reported by: rws Owned by:
Priority: major Milestone: sage-6.9
Component: symbolics Keywords:
Cc: Merged in:
Authors: Ralf Stephan Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: c142a5f (Commits) Commit: c142a5f27a64f5f371d678d33c5facc746d8106b
Dependencies: #18877 Stopgaps:

Description (last modified by rws)

While this works as expected:

sage: n = var('n')
sage: assume(n, 'integer')
sage: solve([n^2 == 3],n)
[]

this does not:

sage: forget()
sage: n = var('n', domain='integer')
sage: solve([n^2 == 3],n)
[n == -sqrt(3), n == sqrt(3)]

So, either solve should also look at the variable domain, or better, let's propagate domain settings to the assumption list. Also:

sage: _ = var('n', domain='integer')
sage: n.is_integer()
False

Creating a non-complex var will invoke Maxima with this. I think this is acceptable for now. Alternatively, examine if assumptions can be done lazily, i.e., before any call to Maxima.

Change History (15)

comment:1 Changed 4 years ago by rws

  • Branch set to u/rws/setting_domain_integer_on_var_has_no_effect_with_solve

comment:2 Changed 4 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to e7fe81a1486be193005eb4243e337cb5a4847f12
  • Status changed from new to needs_review
  • Summary changed from setting domain=integer on var has no effect with solve to propagate the variable domain to the assumptions database

New commits:

e7fe81a18695: propagate the variable domain to the assumptions database

comment:3 Changed 4 years ago by rws

  • Status changed from needs_review to needs_work

comment:4 Changed 4 years ago by git

  • Commit changed from e7fe81a1486be193005eb4243e337cb5a4847f12 to 95f63a61be8893a622e42418b738eaa5fc1968a1

Branch pushed to git repo; I updated commit sha1. New commits:

95f63a618695: fix first attempt; enable exception for wrong domain

comment:5 Changed 4 years ago by rws

  • Status changed from needs_work to needs_review

comment:6 Changed 4 years ago by git

  • Commit changed from 95f63a61be8893a622e42418b738eaa5fc1968a1 to e103b5682b8de062309ce2a1283984f6adc94a78

Branch pushed to git repo; I updated commit sha1. New commits:

e103b5618695: fix indentation

comment:7 follow-up: Changed 4 years ago by rws

  • Dependencies set to #18877
  • Description modified (diff)
  • Status changed from needs_review to needs_work

Together with #18877 this should be able to doctest:

sage: _ = var('n', domain='integer')
sage: n.is_integer()
True

comment:8 Changed 4 years ago by rws

  • Description modified (diff)

comment:9 in reply to: ↑ 7 Changed 4 years ago by rws

Replying to rws:

Together with #18877 this should be able to doctest:

sage: _ = var('n', domain='integer')
sage: n.is_integer()
True

No, this would actually need #18921 and is included there.

Last edited 4 years ago by rws (previous) (diff)

comment:10 Changed 4 years ago by rws

  • Milestone changed from sage-6.8 to sage-6.9
  • Status changed from needs_work to needs_review

comment:11 Changed 4 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to needs_info

Hello,

It looks good. Though I found confusing that the functions sage_domain_to_ginac and sage_domain_to_maxima have the same name. The first one return something (and does nothing) while the second one is communicating with maxima.

comment:12 Changed 4 years ago by git

  • Commit changed from e103b5682b8de062309ce2a1283984f6adc94a78 to c142a5f27a64f5f371d678d33c5facc746d8106b

Branch pushed to git repo; I updated commit sha1. New commits:

d3bf439Merge branch 'develop' into t/18695/setting_domain_integer_on_var_has_no_effect_with_solve
c142a5f18695: clearer names for helpers

comment:13 Changed 4 years ago by rws

  • Status changed from needs_info to needs_review

comment:14 Changed 4 years ago by vdelecroix

  • Status changed from needs_review to positive_review

comment:15 Changed 4 years ago by vbraun

  • Branch changed from u/rws/setting_domain_integer_on_var_has_no_effect_with_solve to c142a5f27a64f5f371d678d33c5facc746d8106b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.