saddle-points: Added efficiency notice about vector of vectors#789
saddle-points: Added efficiency notice about vector of vectors#789petertseng merged 3 commits intoexercism:masterfrom
Conversation
coriolinus
left a comment
coriolinus
left a comment
There was a problem hiding this comment.
Thank you for submitting this PR, @michalfita! It looks generally good to me. I have several suggestions to make the English flow more smoothly, but none are critical enough to block the PR.
I will now leave this PR open for several days to give other maintainers the chance to comment on it. I intend to merge this not later than Monday the 18th. If on Tuesday I still haven't, please feel free to ping me a reminder.
| this exercise is designed to help students understand basic concepts about | ||
| vectors, such as indexing, and that nested data types are legal, _vector of | ||
| vectors_ is a suboptimal choice for high-performance matrix algebra and any | ||
| similar efficient processing of larger amount of data. |
There was a problem hiding this comment.
| similar efficient processing of larger amount of data. | |
| similar efficient processing of larger amounts of data. |
|
@coriolinus Thanks. I approved your suggestions, but that created another commit. I'm not sure if it's acceptable by Exercism rules for PRs? The guide says PR should consist of one commit. |
|
Don't worry about it; if there are a bunch of commits, we can squash-merge
once the PR is finally approved.
…On Wed, Feb 13, 2019 at 9:59 AM Michał Fita ***@***.***> wrote:
@coriolinus <https://github.com/coriolinus> Thanks. I approved your
suggestions, but that created another commit. I'm not sure if it's
acceptable by Exercism rules for PRs? The guide says PR should consist of
one commit.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#789 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHdeTibv4zCKzbFt5a2_GlAWoLOWTsbbks5vM9QHgaJpZM4a4Hnd>
.
|
|
Ups... I need to regenerate locally on my PC. |
|
for PRs that include changes from problem-specifications, please link the relevant PRs. It is exercism/problem-specifications#1429 for this one. When merging, we should include that in the commit message. |
|
@petertseng Is it natural they've been picked up when I updated |
petertseng
added
the
sync/readme
If we assume the tool that was used conforms to the specification of https://github.com/exercism/docs/blob/master/language-tracks/exercises/anatomy/readmes.md#the-readme-template , then that is what will happen.
And, as you may have gotten a hint of with Travis CI, we have a check for that in Travis CI so that we do not get too out of date... therefore any PR that changes saddle-points has to do that, for better or for worse. It's good for making sure nothing accidentally gets missed. |
petertseng
force-pushed
the
787-Saddle_Points_Note
branch
from
f458bfc to
59fb6da
Compare
|
I think it is wiser to separate the problem-specifications changes from the changes made in this PR. So, I have done so. |
petertseng
force-pushed
the
787-Saddle_Points_Note
branch
from
59fb6da to
93dc9a3
Compare
|
Thanks! |
exercism/problem-specifications#1429 which was applied to this track in #789 uses one-based indices in the README. The track still uses zero-based indices in the tests. This inconsistency is a potential source of student confusion. Prevent student confusion by explaining the inconsistency. Closes #791
4 participants
In reference to my issue #787 this is proposed notice in the exercise description about inefficiency of vector of vectors and cache locality aspect.
Update to
README.mdseem to pick up some changes from master exercise repository.I'm open to any suggestions about wording.