Skip to content

Sorting4 #439

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

Merged
merged 6 commits into from
Jun 27, 2021
Merged

Sorting4 #439

merged 6 commits into from
Jun 27, 2021

Conversation

wclodius2
Copy link
Contributor

Changed src/tests/sorting/test_sorting.f90 so that it should run in under one second, by reducing the length of the arrays being sorted, and setting the number of repeats to 1. Changed doc/specs/stdlib_sorting.md so its discussion of the output of test_sorting.f90 reflected the changes in the code. Changed two comments in src/stdlib_sorting.fypp so they were less than 80 columns wide.

Changed sorted integer array from size 2**20 to size 2**16
Changed sorted character array from size 26**4 to 16**4, so the four characters
are a-p.
Changed sorted string array from size 26**3 to 16**3, so the four characters
are a-p.
Change format for the run times from F10.5 to F10.6 to give at least three
digits of precision.

[ticket: X]
Broke up seveeral lines so that the source code would be conistenly no more than
80 columns.

[ticket: X]
Changed the format of a couple of commets so they were less than 80 columns wide.

[ticket: X]
Changes two comments so that they are less than 80 columns wide.

[ticket: X]
I got confused and in my commit of stdlib_sorting.md thought I was committing
stdlib_sorting.fypp. The commit to stdlib_sorting.md actualy documented the
changes in the output of test_sorting.f90, i.e., the reduction in the size of
the arrays being sorted, so the runtime of test_sorting.f90 would be less than
1 second.

[ticket: X]
@wclodius2
Copy link
Contributor Author

I have made the test code, src/tests/sorting/test_sorting.f90, run considerably faster by shortening the arrays and setting repeat to 1. I would appreciate reviews by two reviewers with write access. At least three such reviewers have expressed interest in this code @milancurcic , @jvdp1 , @ivan-pi and also @awvwgk who may not have write access.

@awvwgk awvwgk added the reviewers needed This patch requires extra eyes label Jun 20, 2021
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM. Following the CI, the test for sort takes now < 1 second. I just have a minor comment.

The code had two contexts in which it used the literal value 16.
First for power of two number of elements in the integer arrays.
Second for the number of characters in the character set used to generate
character arrays and strings. For the first conext I defined the parameter
test_power. For the second context I defined the parameter char_set_size,
bot with the value of 16.

[ticket: X]
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Looks good to me

@awvwgk awvwgk removed the reviewers needed This patch requires extra eyes label Jun 27, 2021
@awvwgk awvwgk merged commit 1b93a60 into fortran-lang:master Jun 27, 2021
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

Successfully merging this pull request may close these issues.

3 participants