sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 462 forks source link

do not create optional sage(math)-brial #38182

Open dimpase opened 3 months ago

dimpase commented 3 months ago

36380 goes way too far, for an obviously obsolete piece of code, polybori, which just needs to be purged from Sage. It should not proceed this way.


This (#36380) PR is quite big, it touches a lot of files (the vast majority in a very small way). Is it necessary to do brial and objects and categories all at once? Is there a technical reason this is required?

Originally posted by @kiwifb in https://github.com/sagemath/sage/pull/36380#issuecomment-2157003251

mkoeppe commented 3 months ago

What is this.

kiwifb commented 3 months ago

I feel like my comments have been pulled out of context.

Should brial be purged? Eventually, but proper process is to have a deprecation period before removal. Making a package optional is a nice way around the problem, although not really a proper deprecation process. More of a side-stepping. I would only brutally purge without warning, something that is a security risk or so obsolete that it prevent progress.

dimpase commented 3 months ago

a pile of bugs does not need a superhuge patch touching 616 files.

It's a waste of everyone's time.

kiwifb commented 3 months ago

I am not going to have a shouting match. I am OK with a debate in which we properly address each other points before making our own. At the moment, you are just shouting louder.

With that in mind, can you address my points calmly:

I will add that I inherited the maintenance of brial and can make a release to fix any urgent matters. Brial leaving sage is actually to my benefit as I can then abandon it to anyone wanting it.

dimpase commented 3 months ago

What is this.

Stop your gaslighting efforts. It's unbecoming of an academic who publicly advocates for "inclusivity" and "welcomeness" of SageMath.

dimpase commented 3 months ago

With that in mind, can you address my points calmly:

* Do you propose a plan for deprecation?

It's obvious, although needs a bit of work. BooleanPolynomialRing(...) is a slight extension, qua functionality, of PolynomialRing(GF(2),...). Mathematically/algorithmically, it's a Groebner basis implementation for rings of polynomials over $\mathbb{F}_2$ (i.e. integers mod 2), modulo the ideal generated by the $x_i(x_i+1)$, with the primary (and only?) goal to be used as a SAT solver. It uses tools (BBDs - Binary Decision Diagrams) from the SAT solving toolbox for the inner working, not so surprisingly.

The users of BooleanPolynomialRing can be advised to move to using a SAT solver directly. The only desirable extra functionality is an exporter to a SAT format.

* Is the security of Brial so bad, that it needs immediate removal?

I never said it must be purged now.

* Is the presence of brial preventing something else important to happen in sage?

Packaging for Debian and Fedora?

I will add that I inherited the maintenance of brial and can make a release to fix any urgent matters. Brial leaving sage is actually to my benefit as I can then abandon it to anyone wanting it.

I totally don't see why it cannot be made experimental immediately, and deprecated, too.

mkoeppe commented 3 months ago

I will not participate in this discussion, which is below the necessary standard for respect.

dimpase commented 1 week ago

38182 should not proceed, as it bundles a lot of unrelated things together. @kiwifb - have you positively reviewed it?

dimpase commented 1 week ago

I will not participate in this discussion, which is below the necessary standard for respect.

What kind of "respect" is meant here? mkoeppe just wants to ignore other opinions, that's all what you need to know about "respect" in his interpretation. @kwankyu

mkoeppe commented 1 week ago

Reported as abuse.

kwankyu commented 1 week ago

In my view,

I will not participate in this discussion, which is below the necessary standard for respect.

This is an unfriendly comment, insulting participants in this discussion. A CoC violation according to CoCC.

... mkoeppe just wants to ignore other opinions, that's all what you need to know about "respect" in his interpretation.

Any statement like "M (or D) is such a ... person" is an abusive personal attack. A CoC violation.

mkoeppe commented 1 week ago

In my view,

I will not participate in this discussion, which is below the necessary standard for respect.

This is an unfriendly comment, insulting participants in this discussion. A CoC violation according to CoCC.

@kwankyu You are mistaken. It's recognized as part of best practices to set clear boundaries regarding disrespect. Discussions are simply not possible in such an environment. (If it helps, be reassured that I was for sure not referring to your comments.)

kwankyu commented 1 week ago

...Discussions are simply not possible in such an environment.

There are two other participants (not me) in the discussion. I pointed out how they would feel by your statement.

To be fair, I add that Matthias may not be the first who made unfriendly comments. As always, it is reciprocal...

dimpase commented 1 week ago

I asked a technical question why it's necessary to bundle a lot of unrelated things together.

This was put down as a disrespectful question, and an answer was not given because of this? Who was disrespectful here, let me ask?

Start your own fork of Sage where you can merge your patchbombs as you please, without any reviews.

dimpase commented 4 days ago

a reply to a thread in #36380

In what way are simplify, expand and taylor usable with polynomials?

simplify and expand are no-ops with polynomials. taylor works with polynomials as it does with symbolic expressions.

Note that the current implementation of some of these methods goes through SR. That's a typical modularization obstacle that should be addressed eventually in a follow-up PR.

The problem is not in SR being used, or not. Unless simplify, expand, taylor, polynomials are needed for Sage's type system (which is basically sage.objects + sage.categories) they should not be in. If they are needed, this distribution has even less right to be called sagemath-categories. @mantepse