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

Reals sets consisting of intervals and isolated points #13125

Closed
sagetrac-ares mannequin opened this issue Jun 16, 2012 · 74 comments
Closed

Reals sets consisting of intervals and isolated points #13125

sagetrac-ares mannequin opened this issue Jun 16, 2012 · 74 comments

Comments

@sagetrac-ares
Copy link
Mannequin

sagetrac-ares mannequin commented Jun 16, 2012

Finite unions of open/closed/semi-closed subsets of the real line, for example

    sage: RealSet(0,2) + RealSet.unbounded_above_closed(10)
    (0, 2) + [10, +Infinity)

This ticket also encountered a number of bugs in the comparison of infinity with various field elements like

sage: RLF(0) < oo
False

These are now also fixed and extensive doctests were added.

CC: @kcrisman @tscrim @robertwb

Component: calculus

Author: Volker Braun , Jordi Saludes , Ares Ribó

Branch/Commit: c818746

Reviewer: Ralf Stephan, Peter Bruin

Issue created by migration from https://trac.sagemath.org/ticket/13125

@sagetrac-ares sagetrac-ares mannequin added this to the sage-5.11 milestone Jun 16, 2012
@sagetrac-ares

This comment has been minimized.

@sagetrac-ares

This comment has been minimized.

@kcrisman
Copy link
Member

comment:3

Patch?

@sagetrac-ares
Copy link
Mannequin Author

sagetrac-ares mannequin commented Jun 18, 2012

comment:4

Replying to @kcrisman:

Patch?

uploaded

@vbraun
Copy link
Member

vbraun commented Jun 20, 2013

Initial patch

@vbraun
Copy link
Member

vbraun commented Jun 20, 2013

Changed author from Jordi Saludes and Ares Ribó to Volker Braun

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jun 20, 2013

comment:5

Attachment: trac_13125_real_set.patch.gz

I'm taking over this ticket since I need this for piecewise functions. I'm not sure what happened with the originally proposed patch, but what was attached here is not the actual code.

@vbraun vbraun changed the title Reals sets consisting of intervals and isolated points, supporting integration. Reals sets consisting of intervals and isolated points Jun 20, 2013
@kcrisman
Copy link
Member

comment:7

Is + what we're looking for here? Maybe something like u or U? Just worried about the difference between this "sum" and the Minkowski sum (which is, in fact, already in Sage).

In fact, maybe these should inherit from something related to that... probably not, but just raising the possibility.

How do I get more exotic sets like the union of 1/n for positive integer n? ;-)

@novoselt
Copy link
Member

comment:8

It think it is better to stick to the category of finite unions of intervals and points, then with functions defined on such sets and their periodic extensions (like 1 on [0, 1], 0 on (1,2), then extend periodically in one or both directions to get square wave).

Things with infinite unions lead to accumulation points and cannot really be handled in a uniform way, I suspect.

@kcrisman
Copy link
Member

comment:9

Yes, I was being silly :)

@sagetrac-ares
Copy link
Mannequin Author

sagetrac-ares mannequin commented Jun 22, 2013

Attachment: trac_13125_realsets.patch.gz

Patch for realsets.py

@sagetrac-ares
Copy link
Mannequin Author

sagetrac-ares mannequin commented Jun 22, 2013

comment:10

I just attached the original patch. I did not notice before that the initial patch was not correctly uploaded, I am very sorry for that.

@sagetrac-ares
Copy link
Mannequin Author

sagetrac-ares mannequin commented Jun 22, 2013

Changed author from Volker Braun to Volker Braun , Jordi Saludes , Ares Ribó

@sagetrac-ares

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jun 23, 2013

Attachment: trac_13125_misc.patch.gz

Updated patch

@vbraun
Copy link
Member

vbraun commented Jun 23, 2013

comment:12

I've added the authors to my patch and incorporated any methods that made sense to me. Ares, since your patch would take a bit of work to make use of the Sage class hierarchy and since docstrings are not quite according to Sage specs I propose that we base the implementation on what I have currently posted. The imho only thing left is to decide what to do with the toGf method. Which is some third-party code, I suppose. Maybe you can explain what it is for? If you think it should go into Sage we could turn it into a underscore method.

@vbraun

This comment has been minimized.

@sagetrac-ares
Copy link
Mannequin Author

sagetrac-ares mannequin commented Jun 24, 2013

comment:13

Replying to @vbraun:

I've added the authors to my patch and incorporated any methods that made sense to me. Ares, since your patch would take a bit of work to make use of the Sage class hierarchy and since docstrings are not quite according to Sage specs I propose that we base the implementation on what I have currently posted. The imho only thing left is to decide what to do with the toGf method. Which is some third-party code, I suppose. Maybe you can explain what it is for? If you think it should go into Sage we could turn it into a underscore method.

We will move the toGf method into another paquet, since we use it to support our MOLTO project demo (D6.2.pdf) and it is not directly related to realsets.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@rwst
Copy link

rwst commented Feb 23, 2014

Branch: u/rws/ticket/13125

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

@vbraun
Copy link
Member

vbraun commented May 20, 2014

comment:50

Agree that it should be a separate ticket. It just wasn't clear to me in the beginning that things were this messed up. I've split off the fixing-infinity part into #11506

@tscrim
Copy link
Collaborator

tscrim commented May 20, 2014

comment:51

I let this slip off, sorry Volker. I think there should be no coercion because there is no way to deal with these (semi)infinite intervals. However if we are forced to make this a coercion, then it should be less than infinity since my (very anecdotal) evidence says that most computations won't escape to oo, but instead just don't have a good upper bound.

That being said, I think this should not be positively reviewed as it stands.

Do you think Nils would be a good person to ask about this, someone else, or go directly to sage-devel?

@vbraun
Copy link
Member

vbraun commented May 20, 2014

comment:52

I've moved all infinity-related commits to #11506, let's move the discussion there.

No coercion means: comparison by memory location.

@tscrim
Copy link
Collaborator

tscrim commented May 20, 2014

comment:53

Okay, but the branch currently doesn't merge cleanly

@pjbruin
Copy link
Contributor

pjbruin commented May 20, 2014

comment:54

Just to be clear (since Volker set this to positive review after moving the infinity-related stuff to #11506): it wasn't my intention to give a positive review except for that example. I haven't looked at the real sets code at all.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2014

Changed commit from d7e2c53 to 7b72a42

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2014

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

7b72a42Merge branch 'develop' into t/13125/real_sets

@pjbruin
Copy link
Contributor

pjbruin commented May 20, 2014

comment:57

OK, I didn't notice that the real sets code had already been reviewed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 24, 2014

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

e620d38rm global is_HyperellipticCurve
1932b64rm global is_{Expect,Gap,Singular}Element
3841cc7rm global is_Finite_Field, is_Finite_FieldElement
ae72be9rm global is_Graphics
f0ac47drm global is_ProjectiveSpace
bd2b01arm global is_QuadraticForm
fa34d62rm global is_Scheme
8ad8402rm global is_Morphism
b948874global is_*: delete deprecation warning
c818746fix merge conflict with #14329

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 24, 2014

Changed commit from 7b72a42 to c818746

@vbraun
Copy link
Member

vbraun commented May 25, 2014

Changed branch from u/vbraun/ticket/13125 to c818746

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants