search

wavebitscientific / datetime-fortran

Date and time manipulation for modern Fortran
MIT License
137 stars 51 forks source link

incorrect literal with REAL64 kind #52

Closed wadudmiah closed 6 years ago

wadudmiah commented 6 years ago

The statement below (and similar ones) are incorrect:

real(kind=real64),parameter :: d2s = 86400_real64

the reason being is that 86400 is an integer literal, and the REAL64 kind is for real types. This might work if the REAL64 kind is the same as integer kinds, but will not work otherwise. The safe option is to convert it to real, i.e.

real(kind=real64),parameter :: d2s = 86400.0_real64

I came across this when trying to build it with the NAG Fortran compiler. Might be worth building it with the NAG compiler to catch any other potential bugs.

Cheers, Wadud.

milancurcic commented 6 years ago

Absolutely agree, good catch! Can you submit a PR with this fix?

Thanks, milan

wadudmiah commented 6 years ago

I don't know what PR means. Can you elaborate please? I think you might know Arjen Markus - you can ask him how good the NAG compiler is at catching bugs :-)

milancurcic commented 6 years ago

Can you please fork the repo, commit your changes to fix this in your repo, than submit a Pull Request (PR)? Thanks!

wadudmiah commented 6 years ago

Hi Milan,

Yeah, sure. I can do it this weekend as I'm a bit tied up this week. Hope that's okay. I will run it past the NAG compiler with all error catching flags.

Cheers, Wadud.

wadudmiah commented 6 years ago

Hi Milan, I did a git clone but was unable to create the configure script:

$ ./mkdist.sh 
Running mkdist.sh

Checking versioning consistency
configure.ac reports version: '1.6.1'
datetime.f90 reports version: ''
FAIL: Versions do not match. Aborting
wadudmiah commented 6 years ago

Hi again Milan, could you please let me know the process of pull requests? Thanks.

milancurcic commented 6 years ago

Hi Wadud,

Sorry for the late response. If you git clone, run autoreconf -i and this should generate the configure script for you. Otherwise, you can also build using cmake (see the README).

For contributing, I recommend the following:

  1. Fork the repo. This will get you https://github.com/wadudmiah/datetime-fortran.
  2. git clone https://github.com/wadudmiah/datetime-fortran
  3. Make your code changes locally. Make sure existing unit tests pass, and add unit tests if necessary.
  4. Commit and push to your forked repo on Github.
  5. Hit "New pull request" on your forked repo page on Github.

Thanks!

wadudmiah commented 6 years ago

Hi Milan,

I have created the pull request. Please review the changes. The tests passed. By the way, in your INSTALL file, I would put the command "autoreconf -i" to create the configure script.