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

API Specification for stdlib_kinds #485

Closed
awvwgk opened this issue Aug 18, 2021 · 18 comments · Fixed by #565
Closed

API Specification for stdlib_kinds #485

awvwgk opened this issue Aug 18, 2021 · 18 comments · Fixed by #565
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@awvwgk
Copy link
Member

awvwgk commented Aug 18, 2021

Documentation for the kinds available from stdlib_kinds is currently missing.

@awvwgk awvwgk added the documentation Improvements or additions to documentation label Aug 18, 2021
@awvwgk
Copy link
Member Author

awvwgk commented Aug 18, 2021

We also need to discuss whether we want to be more rigorous and try to support all available kind parameters from iso_fortran_env or generate the kinds using selected_real_kind.

@ivan-pi
Copy link
Member

ivan-pi commented Aug 18, 2021

Previous discussions on this topic include

I think it would be nice to support also the x86 extended 80-bit precision available (real(kind=10)) in generic procedures. There is at-least one place where I could use this now that I'm aware of it.

Personally, I'm kind of against the real32, real64, and real128 for reasons explained by Dr Fortran in "It it takes all KINDS", and believe it would be better if stdlib based it's constants on selected_**_kind.

@zoziha
Copy link
Contributor

zoziha commented Aug 19, 2021

Maybe we can list the required precision type as some options in common.fypp, and use the existing precision (4, 8, 16) by default.
Requires stdlib to use fypp to support(generate) more than 3 precisions, which is a big burden for stdlib before there is no generics and dimension(..) (I believe stdlib_stats will depends on dimension(..)),

@jvdp1
Copy link
Member

jvdp1 commented Aug 19, 2021

Personally, I'm kind of against the real32, real64, and real128 for reasons explained by Dr Fortran in "It it takes all KINDS", and believe it would be better if stdlib based it's constants on selected_**_kind.

While I may have written otherwise in the past, I would be now in favor of using selected_*_kind in stdlib_kind.f90.
Since all modules use stdlib_kinds, changing the definitions of the different kinds should be automically propagated to the whole stdlib.

Adding a new kind (e.g. real(kind=10)) should be quite easy: only stdlib_kinds and common.fypp must be modified.

@jvdp1
Copy link
Member

jvdp1 commented Aug 19, 2021

I just read this interesting thread and comment in Discourse.

@aradi proposed also a possible solution here, which I think could be doable in stdlib: a small Fortran program would generate stdlib_kinds.f90 that depends on hardware/sofware and a common.fypp with the different _KINDS fypp variables.

@awvwgk
Copy link
Member Author

awvwgk commented Aug 19, 2021

We already have a check for real128 in the CMake build files, but this option is not used. I can give it a try and expand this a bit. If I'm allowed to ignore the manual Makefile build I will send a patch for the CMake / fypp setup today or tomorrow.

@jvdp1
Copy link
Member

jvdp1 commented Aug 19, 2021 via email

@awvwgk awvwgk self-assigned this Aug 19, 2021
@milancurcic
Copy link
Member

milancurcic commented Aug 19, 2021

I didn't read the old threads again and I'm not sure that stdlib_kinds was meant for the end-user, but to be used internally by other stdlib modules. It's possible that that's why the spec was omitted. Is there a good end-use case for stdlib_kinds when there's already the constants from iso_fortran_env and selected_real_kind?

@ivan-pi
Copy link
Member

ivan-pi commented Aug 19, 2021

My personal understanding was that it is also for the end user. In all of my personal Fortran projects, I always have a precision module which offers the sp, dp, and wp constants. With stdlib_kinds I thought we ultimately save ourselves from having to rewrite this module again and again, and also settle on common variable names.

But now that you mention the alternative of keeping it for stdlib internal use only, I find it kind of appealing. One of the python zens is "There should be one-- and preferably only one --obvious way to do it". Fortran already has the mechanism with kind and selected_real_kind. On the other hand some of the statistics routine currently return a real(dp) result, meaning dp should probably be available to the end user; or, we must rewrite the documentation to state the return value is kind(1.0d0).

@jvdp1
Copy link
Member

jvdp1 commented Aug 19, 2021

I don't think it make a difference if stdlib_kinds is meant for the end-user, or not.

As mentioned by @kargl 's comment in Discourse I think it would be good that all kinds provided by iso_fortran_env should be at least supported by stdlib. However, the numbers might differ across compiler/os (or even within compilers depending on the chosen options).

If the docs specify how are the different kinds defined in stdlib, then it does not matter if it is available to the user, or not IMO.

@milancurcic
Copy link
Member

@jvdp1 I may not have been clear, I was only commenting on why there may not have been user-facing docs for this module. Of course, users can use the module either way. And of course, it doesn't hurt to write a user-doc, even if meant only for internal use.

@jvdp1
Copy link
Member

jvdp1 commented Aug 19, 2021

@jvdp1 I may not have been clear, I was only commenting on why there may not have been user-facing docs for this module. Of course, users can use the module either way. And of course, it doesn't hurt to write a user-doc, even if meant only for internal use.

Sorry @milancurcic , I misunderstood your comment. IMO there was no specs, because it was one of the first modules developed. The first specs appeared with the function mean if I remember well ;)

@awvwgk
Copy link
Member Author

awvwgk commented Aug 21, 2021

Adding more kind parameters or making one of existing optional adds a problem with the tests. Since the test use the kind parameters explicitly, while the routines in stdlib get generated we have to guard all tests that use kind parameters, which could be optional.

I added fypp support to the CMake macro and realized that this would also be required in the manual Makefile logic. A bit more complicated. It would be nice if we could skip tests, but the current setup of the testsuite doesn't allow this, because the different precision tests are embedded in a single tester and there we can't just use stop 77, because it eliminates the complete test.

Documentation is still something I can do for the current implementation. But to have more kind parameters I think we need a bit of restructuring, which I would submit in separate incremental patches.

@jvdp1
Copy link
Member

jvdp1 commented Aug 22, 2021

Adding more kind parameters or making one of existing optional adds a problem with the tests. Since the test use the kind parameters explicitly, while the routines in stdlib get generated we have to guard all tests that use kind parameters, which could be optional.

fypp could be used for the tests too (at least some of them). I was planning to modify some the stats tests for using fypp. I think that sorting tests could used fypp too

@arjenmarkus
Copy link
Member

arjenmarkus commented Aug 23, 2021 via email

@qin-tain
Copy link

If all kinds supported by compiler need to be considered, the following tiny fortran program may generate all the real kinds supported? Integer kinds can be obtained similarly :)

program main
    implicit none
    integer :: i,k,nk,iu
    integer :: all_kinds(0:50)

    10 format(A, *('"',I0,'"',:,', '))

    open(newunit=iu, file="compiler_kinds.fypp", status="replace", action="write")

    all_kinds = -1; i=0; nk=0
    do
        i = i+1
        k = selected_real_kind(i)
        if (k>0 .and. k/=all_kinds(nk)) then
            nk = nk+1
            all_kinds(nk) = k
        else if (k<0) then
            exit
        end if
    end do
    write(iu, 10, advance="no") "#:set REAL_KINDS = [", all_kinds(1:nk)
    write(iu, "(A)") "]"

    close(iu)
end program

@arjenmarkus
Copy link
Member

arjenmarkus commented Aug 23, 2021 via email

@nshaffer
Copy link
Contributor

nshaffer commented Sep 25, 2021

Why can't we use the arrays defined in iso_fortran_env? Is there a reason we need to dig further than that?

use, intrinsic :: iso_fortran_env, only: real_kinds
implicit none
integer :: u, i

10 format(a, *('"',i0,'"',:,', '))

open (newunit=u, file="compiler_kinds.fypp", status="replace", action="write")
write (u, 10, advance='no') "#:set REAL_KINDS = [", real_kinds
write (u, "(a)") "]"
close (u)
end

This can be extended trivially with integer_kinds, logical_kinds, character_kinds. Have I missed some non-obvious requirement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants