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

Bitsets #236

Closed
wants to merge 134 commits into from
Closed

Bitsets #236

wants to merge 134 commits into from

Conversation

wclodius2
Copy link
Contributor

This branch is intended to provide implementations of bitsets. It defines a parent abstract type bitset_type, and two descendants: bitset_64 for bitsets up to 64 bits in size; and bitset_large, for larger bitsets. The main code is implemented in the module file src/stdlib_bitsets.f90 and its submodule files src/stdlib_bitset_64.f90 and src/stdlib_bitset_large.f90. There are also the test codes src/tests/bistsets/test_stdlib_bitset_64.f90 and src/tests/bistsets/test_stdlib_bitset_large.f90 and a markdown document doc/specs/stdlib_bitsets.md. Various other CMakeLists.txt and Makefile.manual files have been modified so that the code should be compilable with cmake ./ or make -f Makefile.manual.

wclodius2 and others added 30 commits September 1, 2020 16:50
New documentation for the proposed stdlib_logger.f90 module. The documentation differs in style from that of the other modules in the following ways:
1. I thought it benefitted from an introduction giving an overview of the module
2. It has a section describing the constants used as error codes with a table summarizing the constants. The other modules do not appear to have significant public constants
3. It has a section summarizing the derived type. The other modules do not define a public derived type.
4. It has a section describing the `global_logger` variable. The other modules do not define a public variable.
5. It has a table summarizing the various procedures/methods in one place. I think using the module benefits from this summary
6. With the extra material the procedure/method description are one heading higher than in the other documentation. I don't think this is noticeable.
7. The "syntax" has the self argument at an awkward position. The other modules don't have the equivalent of the self argument, and I couldn't see another place to logically put it.
8. The "syntax" follows the standard in using a single pair of square brackets to delimit a run of optional arguments, rather than the other document's use of a pair of square brackets for each optional argument. This is easily changed.
9. The document follows the standard in identifying the class of each procedure.
10. I follow the standard in describing the intent of each argument. The other documentation omits that. This is easily changed.
11. I am wordier than the other documenters. This is hard to change.
12. I ended up upper casing only specifiers that are character strings. I can lower case those, upper case other specifiers or Fortran keywords easily. I just need specific guidance on how to upper/lower case.
The source code for the stdlib_logger.f90 module. It defines one derived type, several constants and procedures/methods to implement loggers, and one variable intended to serve as a global logger.
The test code is quite a bit of a mess. The procedures add_log_file, add_log_unit, and remove_log_unit have failure modes that are modified with a `stat` argument, and need to be tested to be sure the codes do not  wrongly add/remove a unit when the stat argument is not success, and ensure that the stat stat argument returns the correct error code. Any suggestions you have to improve it would be appreciated.
These won't overwrite existing files
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
milancurcic and others added 18 commits September 20, 2020 12:28
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Made the error code name consistent with that used for my bitsets and error_codes modules, where for ease of lookup I put what is invalid before the invalid.
to compile main bitsets files
to compile the main bitsets files
To compile the bitsets test codes
added makefile.manual to compile the two test codes and the test codes test_stdlib_bitset_64.f90 and test_stdlib_bitset_large.f90.
to add bitsets subdirectory.
to add bitsets subdirectory
@jvdp1 jvdp1 mentioned this pull request Sep 28, 2020
Added dependencies of stdlib_bitset_64.o and stdlib_bitset_large.o on stdlib_bitsets.o.
Removed the pure attribute from subroutines that invoked error stop.
Removed the pure attribute from subroutines that invoked error stop.
For bitset_64 changed

integer(block_kind), private, allocatable :: block

to

integer(block_kind), private :: block = 0
@jvdp1
Copy link
Member

jvdp1 commented Sep 29, 2020

@wclodius2
Since the PR on the logger is merged, could you re-open a new PR without all the commits related to the logger? This would help the review.

A solution to do that is to:

  • Create a new branch from your master (e.g., wclodius2:bitsets2)
  • Sync this new branch with fortran-lang/stdlib master
  • Merge your current branch wclodius:bitsets with the new branch wclodius:bitsets2
  • Close the PR related to bitsets and open a new PR from bitsets2. Only the commits from this commit should then appear in the PR.

EDIT: I copied your branch in my repo, and all actions passed!

@wclodius2
Copy link
Contributor Author

@jvdp1 How do I sync a branch with the master? I am not seeing any obvious way to do this on https://github.com/wclodius2/stdlib/tree/bitsets2/src.

@wclodius2 wclodius2 deleted the bitsets branch September 29, 2020 21:32
@everythingfunctional
Copy link
Member

In your case I would recommend the following:

git checkout master
git pull upstream master # or whatever name you've given the remote for the fortran-lang repo
git checkout bitsets2
git merge master

This makes sure you're master branch is up to date with the fortran-lang master branch, and then does a merge commit, bringing any recent changes into your branch. Let me know if you need any additional details.

@wclodius2
Copy link
Contributor Author

I have created a PR for a branch called bitsets3. The PR had one one that failed on ubuntu, out of six attempts, three on ubuntu and three on macOS..

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.

4 participants