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

LinalgRefactor - specialpurpose #3767

Merged
merged 4 commits into from Jun 27, 2017

Conversation

OXPHOS
Copy link
Member

@OXPHOS OXPHOS commented Apr 10, 2017

No description provided.

@OXPHOS OXPHOS added this to In Progress in Linalg refactor Apr 10, 2017
#undef BACKEND_GENERIC_SOFTMAX

/**
* Wrapper method of squared error method.
Copy link
Member Author

Choose a reason for hiding this comment

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

space

@@ -127,6 +133,16 @@ class LinalgBackendEigen : public LinalgBackendBase
DEFINE_FOR_NON_INTEGER_PTYPE(BACKEND_GENERIC_CHOLESKY_SOLVER, SGMatrix)
#undef BACKEND_GENERIC_CHOLESKY_SOLVER

/** Implementation of @see linalg::cross_entropy */
#define BACKEND_GENERIC_CROSS_ENTROPY(Type, Container) \
Copy link
Member Author

Choose a reason for hiding this comment

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

Indent

multiply_by_rectified_linear_derivative_impl(a, result); \
}
DEFINE_FOR_REAL_PTYPE(BACKEND_GENERIC_MULTIPLY_BY_RECTIFIED_LINEAR_DERIV, SGMatrix)
#undef BACKEND_GENERIC_MULTIPLY_BY_RECTIFIED_LINEAR_DERIV
Copy link
Member Author

Choose a reason for hiding this comment

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

non integer real.
Interger doesn't really make sense here.

softmax_impl(a); \
}
DEFINE_FOR_REAL_PTYPE(BACKEND_GENERIC_SOFTMAX, SGMatrix)
#undef BACKEND_GENERIC_SOFTMAX
Copy link
Member Author

Choose a reason for hiding this comment

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

Non integer real

typename SGMatrix<T>::EigenMatrixXtMap a_eig = a;

auto max = a_eig.maxCoeff();
for (index_t j = 0; j < a.num_cols; ++j) //a.num_cols; ++j)
Copy link
Member Author

Choose a reason for hiding this comment

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

remove

@@ -85,6 +85,17 @@ class LinalgBackendViennaCL : public LinalgBackendGPUBase
DEFINE_FOR_ALL_PTYPE(BACKEND_GENERIC_IN_PLACE_ADD, SGMatrix)
#undef BACKEND_GENERIC_ADD


/** Implementation of @see linalg::cross_entropy */
#define BACKEND_GENERIC_CROSS_ENTROPY(Type, Container) \
Copy link
Member Author

Choose a reason for hiding this comment

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

Indent


result.gpu_ptr = std::shared_ptr<GPUMemoryBase<T>>(
result_gpu->clone_vector(result_gpu,a.num_rows*a.num_cols));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

.

@@ -444,6 +444,8 @@ struct cross_entropy<Backend::EIGEN3,Matrix>
}
};



#ifdef HAVE_VIENNACL
Copy link
Member Author

Choose a reason for hiding this comment

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

delete

@OXPHOS
Copy link
Member Author

OXPHOS commented May 5, 2017

Hi @geektoni Could you help me look at my format-check results?
I brew installed clang-format and did git clang-format --commit 0b713865cf064e6a4fdc80cf4aae6c511455b34e --binary /usr/local/bin/clang-format as suggested by travis. I feel like some indentations are moved randomly and so are some back-slashes . Am I missing something?

@geektoni
Copy link
Contributor

geektoni commented May 6, 2017

Hi @OXPHOS. What I found is that:

  • Travis terminal cannot show very well tabs indentation (it's the reason behind the fact that all the backslashes seem to have been moved randomly). If you look at your editor, all the backslashes should be perfectly aligned on the right-most side (mine does);
  • Clang-format evaluate #define without respecting indentation levels (so it will delete all the tabs before it). Moreover, other indentations seem weird because clang-format align the macro calls to the public keyword, which means:
namespace b {
	class a {
		public:

#define BEAUTIFUL_MACRO() /* This won't respect namespace and class indent*/
	void method(...) \
	{
	....	\
	}
		OTHER_MACRO_CALL() /* this is aligned to public keyword below */
#undef BEAUTIFUL_MACRO	/* The same as #define */
	};
}

Unfortunately, I don't think this last behaviour can be fixed easily.

I also see you applied the changes suggested by clang-format, but Travis still fails. Could you please run the same command again? Could you tell me also the clang-format version that you're using? Thanks :)

@OXPHOS OXPHOS force-pushed the linalg_specialpurpose branch 2 times, most recently from a5686eb to 60f81d8 Compare June 5, 2017 04:27
@OXPHOS
Copy link
Member Author

OXPHOS commented Jun 5, 2017

@geektoni Sorry about my late reply..I was away for the last month.

We were talking about that clang-format cannot handle macros very well. My current solution is guard macros with \\ clang-format off - if the backslash alignment is not a hard requirement.

Also, I think that the diffs on travis after I performed clang-format on my machine. My clang-format version is : clang-format version 5.0.0 (tags/google/stable/2017-03-17). I did a brief search and only found information about installing clang-format-3.8 on ubuntu while I am using macbook.

@geektoni
Copy link
Contributor

geektoni commented Jun 5, 2017

Sorry about my late reply..I was away for the last month.

@OXPHOS no problem!

We were talking about that clang-format cannot handle macros very well. My current solution is guard macros with \ clang-format off - if the backslash alignment is not a hard requirement.

Backslash alignment is just to make the code prettier, so it is fine not doing it. However, we should use \\ clang-format off carefully, since if we begin to "protect" code too often, clang-format will become useless ;)

Also, I think that the diffs on travis after I performed clang-format on my machine. My clang-format version is : clang-format version 5.0.0 (tags/google/stable/2017-03-17). I did a brief search and only found information about installing clang-format-3.8 on ubuntu while I am using macbook.

What do you mean here? It seems that a piece of information is missing. Are you talking about installing clang-format-3.8 on OSX?

@OXPHOS
Copy link
Member Author

OXPHOS commented Jun 6, 2017

What do you mean here? It seems that a piece of information is missing. Are you talking about installing clang-format-3.8 on OSX?

@geektoni Yes I was wondering how to install clang-format-3.8 on OSX.
But I might find another way by doing the format checking in Docker. Let me see how it works on Travis.

@OXPHOS OXPHOS force-pushed the linalg_specialpurpose branch 2 times, most recently from 04b35a1 to b3289d1 Compare June 6, 2017 17:38
@karlnapf
Copy link
Member

Can we push this in?

@OXPHOS OXPHOS merged commit 6b0fd29 into shogun-toolbox:develop Jun 27, 2017
@OXPHOS OXPHOS moved this from In Progress to DONE in Linalg refactor Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants