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

CpuMath Enhancement: Rename input arguments of CpuMath SSE/AVX intrinsics #824

Closed
briancylui opened this issue Sep 5, 2018 · 3 comments
Closed
Labels
up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@briancylui
Copy link
Contributor

briancylui commented Sep 5, 2018

Style changes needed to solve part of #823

Details

  • Rename input arguments of methods in src\Microsoft.ML.CpuMath\CpuMathUtils.netcoreapp.cs and src\Microsoft.ML.CpuMath\Sse.cs, to make them more informative, e.g.:
  • for MatTimesSrc, change tran into transpose

  • for a and b, change them into src and dst

@briancylui briancylui changed the title Rename input arguments of CpuMath SSE/AVX intrinsics CpuMath Enhancement: Rename input arguments of CpuMath SSE/AVX intrinsics Sep 6, 2018
@danmoseley danmoseley added the up-for-grabs A good issue to fix if you are trying to contribute to the project label Sep 6, 2018
@jwood803
Copy link
Contributor

Hey, @briancylui. I was going to mess with this issue, as well.

I did notice a few methods have parameter names of src as well as a name of a. Is there something else to name a to in this case?

public static void Scale(float a, float[] src, float[] dst, int count)

@briancylui
Copy link
Contributor Author

@jwood803: Thanks for your kind help! According to previous PR reviews, there were voices to change a into scale, but not so sure about src and dst. Adding @eerhardt and @tannergooding since they also raised new naming suggestions earlier.

@eerhardt
Copy link
Member

I would just spell the words out - source and destination.

@shauheen shauheen closed this as completed Dec 6, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

No branches or pull requests

5 participants