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

RangeSet: support negative ranges (#515) #518

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

thiell
Copy link
Collaborator

@thiell thiell commented Feb 4, 2023

Support negative ranges in RangeSet, with as little impact as possible to the normal (positive range) codepath. Zero padding is not available with negative ranges at this time for this reason.

In case of negative ranges, RangeSet will case the numbers to integer. Internal sorting then looks like this (eg. -5-6):
['-5', '-4', '-3', '-2', '-1', '0', '1', '2', '3', '4', '5', '6']

Apart from the internal sorting change, little modifications are needed in the parser and the folding method, so I guess it's worth it.

Usage examples:

>>> RangeSet(range(-5,7)) - RangeSet(range(1,3))
-5-0,3-6

>>> list((RangeSet(range(-5,7)) - RangeSet(range(1,3))).intiter())
[-5, -4, -3, -2, -1, 0, 3, 4, 5, 6]

>>> list((RangeSet(range(-5,7)) - RangeSet(range(1,3))).contiguous())
[-5-0, 3-6]

Hence, the following is now possible:

$ cluset -f foo[-10-10]
foo[-10-10]

$ cluset -f foo[-100--90/2]
foo[-100,-98,-96,-94,-92,-90]

Closes #515.

Support negative ranges in RangeSet, with as little impact as
possible to the normal (positive range) codepath. Zero padding is not
available with negative ranges at this time for this reason.

In case of negative ranges, RangeSet will case the numbers to integer.
Internal sorting then looks like this (eg. -5-6):
['-5', '-4', '-3', '-2', '-1', '0', '1', '2', '3', '4', '5', '6']

Apart from the internal sorting change, little modifications are needed
in the parser and the folding method, so I guess it's worth it.

Usage examples:

    >>> RangeSet(range(-5,7)) - RangeSet(range(1,3))
    -5-0,3-6

    >>> list((RangeSet(range(-5,7)) - RangeSet(range(1,3))).intiter())
    [-5, -4, -3, -2, -1, 0, 3, 4, 5, 6]

    >>> list((RangeSet(range(-5,7)) - RangeSet(range(1,3))).contiguous())
    [-5-0, 3-6]

Hence, the following is now possible:

$ cluset -f foo[-10-10]
foo[-10-10]

$ cluset -f foo[-100--90/2]
foo[-100,-98,-96,-94,-92,-90]

Closes cea-hpc#515.
@thiell thiell added this to the 1.9.1 milestone Feb 5, 2023
@thiell thiell self-assigned this Feb 5, 2023
Copy link
Collaborator

@degremont degremont left a comment

Choose a reason for hiding this comment

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

That looks great (negative value support is creating some crazy rangeset patterns :p )

Any concern on the perf impact, mostly for sorted?

@thiell
Copy link
Collaborator Author

thiell commented Feb 5, 2023

Thanks for the quick review! No real concern here w.r.t. performance as the sorting method for positive ranges remains the same, on the same kind of tuples (len(x), x), although it's true that there is a if condition added to check for -. But after a quick test (in Python 2 only, btw we should port the benchmarks to Python 3 to be more relevant nowadays), there nothing really significant I can detect:

[sthiell@dev-centos7 bench.py2_neg-ranges]$ ./UnitBench.py compare master b191_515
 TESTNAME                 master   b191_515
 NodeSet15Add            0.08578    0.08793        3%
 NodeSet15Contains       0.09407    0.09806        4%
 NodeSet15Iter           0.00115    0.00136       18%
 NodeSet15Parse          0.07032    0.07151        2%
 NodeSet15Pickle         0.00579    0.00621        7%
 NodeSet15Remove         0.14756    0.15284        4%
 NodeSet15Str            0.00421    0.00440        4%
 NodeSet15Union          0.00019    0.00019       -3%
 NodeSet2DAdd            0.08004    0.07899       -1%
 NodeSet2DContains       0.39155    0.41084        5%
 NodeSet2DIter           0.13716    0.14571        6%
 NodeSet2DParse          0.40551    0.40850        1%
 NodeSet2DRemove         1.69130    2.02133       20%
 NodeSet2DStr            0.13921    0.15779       13%
 NodeSet2DUnion          0.15036    0.15690        4%
 NodeSet2DcPickle        0.05444    0.05821        7%
 NodeSetAdd              0.04538    0.04167       -8%
 NodeSetContains         0.04776    0.04288      -10%
 NodeSetIter             0.00109    0.00117        7%
 NodeSetParse            0.03697    0.03408       -8%
 NodeSetRemove           0.05286    0.04723      -11%
 NodeSetStr              0.00444    0.00416       -6%
 NodeSetUnion            0.00012    0.00010      -16%
 NodeSetcPickle          0.00582    0.00556       -4%
 RangeSetAdd             0.00179    0.00181        1%
 RangeSetContains        0.00158    0.00178       13%
 RangeSetIter            0.00073    0.00092       26%
 RangeSetParse           0.00763    0.00794        4%
 RangeSetRemove          0.00331    0.00326       -2%
 RangeSetStr             0.00420    0.00431        3%
 RangeSetUnion           0.00009    0.00009       -0%
 RangeSetcPickle         0.00538    0.00572        6%

@thiell thiell merged commit 8059dd1 into cea-hpc:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for negative ranges
2 participants