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

Improve validation in loop_interchange_1 #7

Open
dendibakh opened this issue Aug 16, 2021 · 6 comments
Open

Improve validation in loop_interchange_1 #7

dendibakh opened this issue Aug 16, 2021 · 6 comments

Comments

@dendibakh
Copy link
Owner

Comment by @OleksandrKvl :

Current validation is strange in a way that you're using other functions from solution.cpp, not just the main power(). Ideally, you should have separate implementation inside validate.cpp, multiple some matrices with functions from validate.cpp and solution.cpp and just compare their results.

@dendibakh
Copy link
Owner Author

cc @andrewevstyukhin

@andrewevstyukhin
Copy link
Collaborator

Often in real cases not all code paths are covered. Any optimization can easily break something. It's a good honeypot in a too simple task to show reality.
Anyway, commit 285f007 prevents trival broken solution {}.

@dendibakh
Copy link
Owner Author

Often in real cases not all code paths are covered. Any optimization can easily break something. It's a good honeypot in a too simple task to show reality.
Anyway, commit 285f007 prevents trival broken solution {}.

So Andrey, do you suggest that we don't duplicate code from solution.cpp into validate.cpp ?

@andrewevstyukhin
Copy link
Collaborator

I think we can use originalMultiply in validation.cpp to be more precise if current situation is so dramatic.
Please try 285f007 first (just merge main into private/*)

@OleksandrKvl
Copy link
Collaborator

The problem is not just a validation, it's also about clear separation between client's and core code. Originally, I edited multiply() and identity() functions in my solution and was very surprised to find that they are used by someone else. I think that it should be clear what people can and cannot do. The simplest approach is to allow them to change only solution.h/cpp. For example existing solution.h:

// Assume this constant never changes
constexpr int N = 400;

// Square matrix 400 x 400
using Matrix = std::array<std::array<float, N>, N>;

void zero(Matrix &result);
void identity(Matrix &result);
void multiply(Matrix &result, const Matrix &a, const Matrix &b);
Matrix power(const Matrix &input, const uint32_t k);

void init(Matrix &matrix);

Should be just:

#include "problem.h" // Matrix and N are here now

Matrix power(const Matrix &input, const uint32_t k);

@andrewevstyukhin
Copy link
Collaborator

Performance problems lay in solution.h and solution.cpp. Microoptimizations don't mean to use other algorithm or rewrite source code. Location of the performance issue is also unknown. Good job is to use information from the topic and use a profiler. All issues are topmost, not second order.

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

No branches or pull requests

3 participants