Skip to content

Conversation

@michel-steuwer
Copy link
Member

@michel-steuwer michel-steuwer commented Aug 20, 2021

This is a draft PR porting shine and all dependent repositories to Scala 3.

At the moment of opening this PR, there exists a large performance regression that me need to address before merging.

Here are the changes made in:

@Bastacyclop
Copy link
Member

I see that you have changed some lazy vals to defs, could that be part of the performance regression?

@Bastacyclop
Copy link
Member

Bastacyclop commented Aug 23, 2021

In elevate macros you are wrapping these everywhere (strategies, combinators):

       try ... catch
          case _: MatchError => Failure(this)

I don't think this is what we used to do?
Ideally Success/Failure should only be automatically wrapped around rules and not strategies/combinators (i.e. automatically lifting partial rule functions to monadic functions)?

@Bastacyclop
Copy link
Member

In src/main/scala/elevate/core/strategies/debug.scala, there are rule definitions which look more like strategies to me. I think we should be careful about what is a rule and what is a strategy?

) `;`
reducedFusedForm `;`
normalize.apply(
normalize(
Copy link
Member

Choose a reason for hiding this comment

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

finally :)

@@ -1,18 +1,20 @@
package apps

import apps.separableConvolution2D._
Copy link
Member

Choose a reason for hiding this comment

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

import apps.separableConvolution2D.*, or delete?

import rise.core.Expr
import shine.OpenCL._
import shine.cuda.KernelExecutor.{KernelNoSizes, KernelWithSizes}
import util._
Copy link
Member

Choose a reason for hiding this comment

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

what is the difference between import x._ and import x.*?

Base automatically changed from scala-3-prep to main March 21, 2025 13:22
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.

3 participants