Skip to content

Conversation

@volkm
Copy link
Contributor

@volkm volkm commented Dec 3, 2025

Addresses some parts of #831

@sjunges
Copy link
Contributor

sjunges commented Dec 13, 2025

LGTM, but why not use isLess with precision (or rather: tolerance) 0?

@volkm
Copy link
Contributor Author

volkm commented Dec 15, 2025

My main reason was that isLess requires a ConstantsComparator. But I agree that it would be more clear to use isLess or even better isLessEqual.
Maybe a more general question: when should we use constants.h and when ConstantsComparator? It seems that both overlap in some functionality.

@sjunges
Copy link
Contributor

sjunges commented Dec 22, 2025

I would say that the comparator is only used when you have a state, i.e., the type of precision.

Maybe we should extend the constants to have a definition of isLess that does not require any precision?

@volkm
Copy link
Contributor Author

volkm commented Jan 7, 2026

I agree with your idea to add isLess (without precision) to constants.h. I would simply use the < operator here.
I took a look at ElementLess which we already define and which also uses std::less for comparison. I suggest to remove the double specialization though because it has two issues: a magic number for precision and it only works for non-negative numbers.

@sjunges
Copy link
Contributor

sjunges commented Jan 9, 2026

I think we haven't used < because the code has to also work for polynomials etc and the intended semantics of the < operator is not clear?

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.

2 participants