scalanlp / breeze

Breeze is a numerical processing library for Scala.
www.scalanlp.org
Apache License 2.0
3.44k stars 691 forks source link

Wip update spire #833

Closed Quafadas closed 2 years ago

Quafadas commented 2 years ago

This aims to remove the 2_13 dependancy on spire. Spire itself was compiled with 3.1.0 as far as I could tell, so I also bumped breezes scala version.

That generated a bunch of problems. Upgrading the collection-compact modules, apparently fixed these problems.

With these changes, the tests in the "math" sub project passed (for me).

Health Warnings: Spire release is "M2". It may be preferable to wait for a 0.18.0 As I don't have a detailed understanding of Breeze, these changes may not be desirable in some larger context I'm not aware of.

dlwh commented 2 years ago

Thanks! I have a slight preference for supporting 3.0.x, but if that's what Spire can support, than probably best to bump. I agree it's best to wait for a full release or at least RC before we cut a release with it.

armanbilge commented 2 years ago

Just had a spire user run into the dreaded Conflicting cross-version suffixes trying to use spire and breeze together on Scala 3 and found my way here. Thanks for working on this!

I have a slight preference for supporting 3.0.x

Thanks, this is helpful feedback. AFAIK nothing in spire depends on Scala 3.1.x (but can't be sure), so we can try rolling back if it's important to you. This issue has been controversial :) We could also wait and see about https://github.com/lampepfl/dotty/pull/14156, I think they originally promised it for 3.1.1 but I haven't followed closely.

I agree it's best to wait for a full release or at least RC before we cut a release with it.

We released an M3 last week. I don't anticipate any more breaking changes going forward, we can probably RC or go straight to final.

romanowski commented 2 years ago

lampepfl/dotty#14156 will be released in 3.1.2. RC1 should be released shortly and a stable version 6 weeks later (unless there are no critical bugs).

Swoorup commented 2 years ago

Likely also need to change: https://github.com/scalanlp/breeze/blob/master/macros/build.sbt#L10

dlwh commented 2 years ago

sorry for being slow. Yeah my preference for 3.0.x was driven by the lack of forwards compatibility, but if they're going to fix that, then that's great.

dlwh commented 2 years ago

oops. shouldn't merge things when i have a migraine

dlwh commented 2 years ago

(reverted the merge in master. don't know how to reopen a PR)

armanbilge commented 2 years ago

sorry for being slow. Yeah my preference for 3.0.x was driven by the lack of forwards compatibility, but if they're going to fix that, then that's great.

@dlwh do you still feel the same way about sticking to Scala 3.0.x? I think Spire will go straight to Scala 3.1. I experimented with the new -scala-output-version flag but it seems we've already used some Scala 3.1 APIs in macros so it doesn't work. I'd rather not backport these.

[error] -- Error: /workspace/spire/core/src/main/scala-3/spire/syntax/macros/literalMacros.scala:35:14 
[error] 35 |  parseNumber(digits.valueOrAbort.parts, BigInt(-128), BigInt(255)) match
[error]    |              ^
[error]    |method valueOrAbort was added in Scala release 3.1, therefore it cannot be used in the code targeting Scala 3.0

See some discussion on the topic in:

dlwh commented 2 years ago

i don't care that much anymore I guess, so fine with me!

On Thu, May 26, 2022 at 2:21 PM Arman Bilge @.***> wrote:

sorry for being slow. Yeah my preference for 3.0.x was driven by the lack of forwards compatibility, but if they're going to fix that, then that's great.

@dlwh https://github.com/dlwh do you still feel the same way about sticking to Scala 3.0.x? I think Spire will go straight to Scala 3.1. I experimented with the new -scala-output-version https://github.com/sbt/sbt/releases/tag/v1.7.0-M2 flag but it seems we've already used some Scala 3.1 APIs in macros so it doesn't work. I'd rather not backport these.

[error] -- Error: /workspace/spire/core/src/main/scala-3/spire/syntax/macros/literalMacros.scala:35:14 [error] 35 | parseNumber(digits.valueOrAbort.parts, BigInt(-128), BigInt(255)) match [error] | ^ [error] |method valueOrAbort was added in Scala release 3.1, therefore it cannot be used in the code targeting Scala 3.0

See some discussion on the topic in:

— Reply to this email directly, view it on GitHub https://github.com/scalanlp/breeze/pull/833#issuecomment-1139070682, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACLIPCES5G4CBUYK5ZEHTVL7TOBANCNFSM5K7PJG7A . You are receiving this because you were mentioned.Message ID: @.***>