Mentions légales du service

Skip to content
Snippets Groups Projects

Starpu/cublas v2

Merged Mathieu Faverge requested to merge faverge/chameleon:starpu/cublas_v2 into master

This pull request introduce the API v2 supports to enable additional kernels such as geam cited in issue #22 (closed) @all This doesn't take into account magma kernels yet, but I thing we should stop their support since it is not up to date. Unless @fpruvost says they are still working.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • I would say nothing. We never use them, and it was used only for hybrid kernels (such as panel factorization) and that we should remove for simplicity. So if everyone agrees with:

    1. I'll merge this request when we are sure everything is compiling and working
    2. I'll do another pull request to remove deprecated kernels from magma
  • I disagree. Let me please explain why.

    I fully understand that from both a software point of view and a performance point of view, this dependency is annoying. It does not bring speed up in most cases and it complicates the installation process.

    On the other hand, Chameleon is not a solver whose only aim is performance. It is also a platform being used for designing and studying scheduling algorithms. We want to be able to study the acceleration brought by panel factorizations on GPUs, even if in the end it is for saying it limits the performance.

    Keeping an optional dependency on Magma disabled by default would be great.

  • I agree with your argument, and I could agree with you on keeping this code as an option IF:

    • the code was compiling
    • the code was numerically correct,
    • the code was maintained with new magma releases

    But this code has not been maintained for years now, it doesn't work (and I'm not speaking about performance) , and I'm really fully against keeping trash code in the master branch even as an option, because users always tries options :P. Either it compiles and works (even with slow performance) and it can be kept in the master branch, otherwise it belongs to personal forks. I think that neither @fpruvost nor I will fix it and maintain it, so unless you volunteer for the job, I'm in favor of putting this code to trash.

    And for now, since we are in a democracy, we have two votes against one :D

    By the way, are you against this discussion or against this merge request ?

  • Maybe, rather than putting to trash, putting it to a branch, to increase the chance that some people find it and fix it.

  • I only disagree on the point that we should send it to trash without caution. Because we might loose it forever.

    There are different ways. As @thibault is proposing, putting it into a branch is for instance a nice option. Would that be fine with you @faverge ?

  • I completely agree that it should be kept somewhere. Just not in the master :).

    So, after discussion we created a tag version bugged/gpupanel to be able to come back to a version where those kernels exist. Once again, just to be clear, the point is just to remove the kernels and the magma support from the computation side since it's bugged. We keep the simulation model for SimGrid as people are using them.

  • @sukkarde have you been able to try this pull request ? to check it was correct ?

  • Mathieu Faverge added 23 commits

    added 23 commits

    • d4c93f6d...f7e10e33 - 17 commits from branch solverstack:master
    • e7d20b3b - Add a mavcro to switch from stream to handle
    • 117a67f9 - Use the new macros
    • f41be856 - Setting the kernel is done in the codelet
    • e50caa38 - Always include cublas before cublas_v2
    • d363ff0a - Upgrade kernels to compile with API v2 (even with this crazy change in TRSM API .....)
    • 8617f58a - Add option to enable V2 by default with StarPU

    Compare with previous version

  • @sukkarde said it was working, so since I don't see anything against this pull request I merged and I hope we will be able to use the new interface easily :)

  • Mathieu Faverge mentioned in commit e229fd74

    mentioned in commit e229fd74

Please register or sign in to reply
Loading