Min/Max now supports sets as argument

[oscargus]

References to other Issues or PRs

Closes #22568

Brief description of what is fixed or changed

It is now possible to use sets as arguments to Max and Min. I needed a way to write something like

Max(imageset(Lambda(x, f(x), Range(J))

and later on replace J with an integer.

Although this is not yet supported as it doesn't seem possible to convert Range to FiniteSet at will, see #22571, it is a step in the right direction.

Other comments

Release Notes

• functions
  • Max and Min now accept Set arguments.

[sympy-bot]

✅

Hi, I am the SymPy bot (v162). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• functions
  • Max and Min now accept Set arguments. (#22572 by @oscargus)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.10.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

Closes #22568

#### Brief description of what is fixed or changed
It is now possible to use sets as arguments to `Max` and `Min`. I needed a way to write something like

`Max(imageset(Lambda(x, f(x), Range(J))`

and later on replace J with an integer.

Although this is not yet supported as it doesn't seem possible to convert `Range` to `FiniteSet` at will, see #22571, it is a step in the right direction.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* functions
    * `Max` and `Min` now accept `Set` arguments. 
<!-- END RELEASE NOTES -->

[github-actions]

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [907895ac]       [c551272c]
+      7.43±0.3ms       11.2±0.8ms     1.50  matrices.TimeMatrixPower.time_Case1
-      4.94±0.03s          357±6ms     0.07  polygon.PolygonArbitraryPoint.time_bench01
+      4.09±0.1ms       6.70±0.4ms     1.64  solve.TimeMatrixOperations.time_det(4, 2)
+      3.95±0.3ms       6.37±0.1ms     1.61  solve.TimeMatrixOperations.time_det_bareiss(4, 2)
+        40.9±2ms       69.4±0.8ms     1.70  solve.TimeMatrixSolvePyDySlow.time_linsolve(1)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

[smichr]

I think this might be conflating ideas of objects and object elements. In a similar way we needed to deistinguish between FiniteSet(FiniteSet(1, 2)) != FiniteSet(1, 2). When the user writes Max(a, b) then a and b must be compared, not the elements within. If there is code that needs to find the largest item in sets then it should be max(i.sup for i in sets).

[smichr]

I think max(Range) is ok but have doubts about max(Range, FiniteSet). Note how the builtin behaves:

>>> max([1,2,3],[2])
[2]

So it returned the element that had the largest first difference. And mixed comparisons fail:

>>> max([1,2,3],[2],7)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '>' not supported between instances of 'int' and 'list'

[oscargus]

OK! I can see the point of not allowing Max(set1, set2) (as there is no way to compare sets afaIk). However, from a mathematical perspective, the MaxSet function is, well, max. So it will print exactly like max (although ideally for an ImageSet with the domain below the max).

So I think that it does make sense to have a "max"-function which allows a single set (and, hence, takes the maximum set member). If that then leads to a different class being executed, so be it. More work for limited benefit.

(This means that while Max(set1, set2) is not allowed but Max(Max(set1), Max(set2)) is and set1.sup can return Max(set1) if nothing else is available.)

The whole point is to be able to manipulate these types of expressions without having to do additional coding, but to benefit from the built-in support. As an added benefit it will generate mathematically correct/valid output for intermediate steps.

[smichr]

What about max(Union(*sets)) or max(i.sup for i in sets)? I pefer the latter. And even though you don't know of a meaningful way to compare sets, Python has chosen a way to compare iterables. Since we are in Python, we might aim for the 'least surprise' result.

[smichr]

Note, too, that FiniteSet can hold FiniteSets, so even max(FiniteSet) would have to decide what the biggest element is when comparing the two sets, for example, in FiniteSet({1,2},{3,4}). It just doesn't seem like max(*sets) makes sense; and at least the example I gave recognizes that one can't compare the two sets directly and raises a Value Error when requesting the .sup of that set of sets.