Skip to content

fix #281 ("MDTest calculation of multiple iterations was incorrect") for 3.3 release branch #299

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 22, 2020
Merged

fix #281 ("MDTest calculation of multiple iterations was incorrect") for 3.3 release branch #299

merged 3 commits into from
Dec 22, 2020

Conversation

glennklockwood
Copy link
Contributor

A reproducer wasn't included with #281 so I haven't tested to make sure this is complete, but the patch in #281 seemed pretty straightforward.

This should resolve #298.

@glennklockwood glennklockwood added this to the 3.3.0 milestone Dec 19, 2020
@glennklockwood
Copy link
Contributor Author

Also if/when we merge this, can we squash+merge? I noticed a couple of pull requests weren't squashed before merging which have polluted our commit history.

JulianKunkel
JulianKunkel reviewed Dec 19, 2020
View reviewed changes
Copy link
Collaborator

@JulianKunkel JulianKunkel left a comment

Choose a reason for hiding this comment

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

On the new Line 1301, need to replace all[....] with all[calc_allreduce_index(j, k, i)] as well. Otherwise, looks good.

@glennklockwood
Copy link
Contributor Author

glennklockwood commented Dec 21, 2020 •
edited
Loading

On the new Line 1301, need to replace all[....] with all[calc_allreduce_index(j, k, i)] as well. Otherwise, looks good.

I was wondering about that. Does this mean that the following patch (on the 3.2 branch) should also be changed?

ior/src/mdtest.c

Line 1510 in 7a490c8

var += pow((mean - all[(k*tableSize*iterations)

Without a reproducer I can't easily tell what the effects are (also because I'm too lazy to refamiliarize myself with the mdtest code).

@glennklockwood glennklockwood self-assigned this Dec 22, 2020
@glennklockwood glennklockwood merged commit e92a0cd into hpc:3.3 Dec 22, 2020
@glennklockwood
Copy link
Contributor Author

Without a reproducer I can't easily tell what the effects are (also because I'm too lazy to refamiliarize myself with the mdtest code).

Nevermind. Unless someone objects, I'll leave 3.2 alone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JulianKunkel JulianKunkel JulianKunkel left review comments

Assignees

@glennklockwood glennklockwood

Labels
None yet
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@glennklockwood @JulianKunkel