Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scala 3 Draft PR #205

Draft
wants to merge 4 commits into
base: scala-3-prep
Choose a base branch
from
Draft

Scala 3 Draft PR #205

wants to merge 4 commits into from

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?

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?

@@ -6,6 +6,8 @@ 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.*?

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