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

add system module #54

Merged
merged 2 commits into from
Jan 8, 2020
Merged

add system module #54

merged 2 commits into from
Jan 8, 2020

Conversation

scivision
Copy link
Member

@scivision scivision commented Dec 30, 2019

There are a number of capabilities it would be useful to bring from
cstdlib and STL. This is an initial demonstration, replacing
the non-cross-compiler sleep() with a standard implementation that
works across compilers and operating systems, with millisecond integer
input.
For example, Intel compilers can hang at runtime if call sleep() is used without use ifport. This PR technique doesn't suffer from that issue across operating systems and compilers.

There are a number of other useful functions that can be implemented like this, including resolving absolute filenames and expanding ~ to user home path

Adding CI for Windows brought an interesting corner case to light--runtime segfaults on Windows. I don't see these on my Windows Gfortran PC. This is a common type of corner case, tricky/shaky tests using uncommon features where it isn't feasible to whitelist/blacklist systems. A future PR would do a "check_fortran_source_run()" on a snippet of code to ensure the program can run instead of nuisance failure.

@certik
Copy link
Member

certik commented Dec 30, 2019

Thanks @scivision for pushing this. Can you create another PR where you get the Windows build working? I am working on reviewing #51 now, let's get that one in first.

I like what you are trying to do, and let's get it polished an in.

@scivision
Copy link
Member Author

scivision commented Dec 30, 2019

Yes basically the CMake needs #51 to get working where I could put the necessary Windows stuff if. For these Windows tests, it will require CMake >= 3.14 to do the workaround. I added the feature to CMake 3.14 that makes it possible.

@certik
Copy link
Member

certik commented Dec 30, 2019

@scivision in that case we can depend on CMake 3.14, which I know @zbeekman would be very happy about. :)

@zbeekman
Copy link
Member

@scivision in that case we can depend on CMake 3.14, which I know @zbeekman would be very happy about. :)

I told you there be bugs... all's well that ends well.

Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

I would strongly suggest to split the unrelated and approved parts of this PR into separate PRs, so that we can merge them:

  • The Windows CI tests
  • The CMake changes
  • The action argument

The actual system module will require some discussion about the API, before we can merge.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

I agree with @certik to use smaller and more focused PRs. We can narrow down the workflow details together in #5.

Reviewed here only the system sleep additions. I think they're in scope for stdlib and useful. We can continue discussing and working out the details here, but for any additions to the API, please open an issue and discuss there before moving forward to a PR.

@scivision scivision force-pushed the systemlib branch 2 times, most recently from a0bdf78 to 9ec1846 Compare December 31, 2019 17:02
@zbeekman
Copy link
Member

I would strongly suggest to split the unrelated and approved parts of this PR into separate PRs, so that we can merge them:

I'll have a go at this in a moment.

@certik
Copy link
Member

certik commented Jan 3, 2020

@scivision most of your other unrelated improvements in this PR have been merged into master. Do you want to submit a clean PR with just the system module?

@scivision scivision force-pushed the systemlib branch 3 times, most recently from 7fbbba0 to c513abf Compare January 5, 2020 02:42
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

That looks good to me.

@scivision scivision force-pushed the systemlib branch 2 times, most recently from 688cb98 to e740426 Compare January 5, 2020 04:30
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

Overall it looks good. I left some minor comments.

@certik
Copy link
Member

certik commented Jan 6, 2020

@scivision I would like to merge your system module, but when you bundle unrelated CI changes, I don't want to just merge it, because some people might disagree with the CI changes, or want to further discuss them.

Do you think it would be please possible to not bundle unrelated changes in a single PR?

Let's split this PR, so that we can merge the system module, which I think is a great start and exactly as you say, it would allow us to use this approach for other things.

@scivision scivision force-pushed the systemlib branch 2 times, most recently from 67a3b8a to 7dcbff9 Compare January 6, 2020 16:00
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

As the PR stands right now, I think I am ok with merging it. It shows how to use system API from Fortran, and we can use it for all the other things that we want to do for the os module.

@zbeekman, @milancurcic if you could both also review it before we merge, that would be great.

Copy link
Member

@zbeekman zbeekman left a comment

Choose a reason for hiding this comment

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

I'd love to see module and program scopes indendented, and single new-lines at EOF, but these are nits and will be irrelevant if/when auto-formatting is implemented. Otherwise, looks good to me.

@certik
Copy link
Member

certik commented Jan 6, 2020

Let's not worry about formatting too much in individual PRs --- rather let's figure out how to do this automatically for all source files.

@certik
Copy link
Member

certik commented Jan 6, 2020

@zbeekman thanks for the review!

@milancurcic can you please review this also?

For new functionality, I want several independent reviews, to prevent putting things in that might not be well designed or too bloated.

There are a number of capabilities it would be useful to bring from
cstdlib and STL. This is an initial demonstration, replacing
the non-cross-compiler sleep() with a standard implmeentation that
works across compilers and operating systems, with millisecond integer
input.
@certik
Copy link
Member

certik commented Jan 8, 2020

@milancurcic can you please review this also?

@scivision sorry for the delay.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Great! Thank you @scivision, and sorry for the delay with me reviewing this. I missed the notifications.

@milancurcic milancurcic merged commit f300f4a into fortran-lang:master Jan 8, 2020
@scivision scivision deleted the systemlib branch January 8, 2020 23:53
jvdp1 added a commit to jvdp1/stdlib that referenced this pull request Jan 21, 2020
Squashed commit of the following:

commit 3266163
Merge: e96c12d 4274f0d
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 20:47:32 2020 +0100

    modification of CMake and Makefile

    Merge branch 'stat_cmake' into stat_dev

commit 4274f0d
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 20:44:24 2020 +0100

    stat_cmake: update Makefile

commit 17e3d16
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 20:35:19 2020 +0100

    second try cmake

commit 397eb18
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 20:18:37 2020 +0100

    Modifications of CMake for tests on Ubuntu 7

commit e96c12d
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 19:40:05 2020 +0100

    small change in md

commit 7eec9ae
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 19:37:06 2020 +0100

    stat_dev: renamed stat to stats

commit 8199b6d
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 19:26:20 2020 +0100

    stat_dev: changed spec

commit b1c481d
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 19:15:25 2020 +0100

    stat_dev: modifs following comments

commit e64657c
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 14:23:59 2020 +0100

    stat_dev: addition of .md file for mean

commit ad504e8
Merge: 5a1adcb bab50e3
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 13:16:21 2020 +0100

    Merge remote-tracking branch 'jvdp1/stat_dev_1' into stat_dev

commit bab50e3
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 13:13:00 2020 +0100

    stat_dev_1: changed all to iterations

commit 8d4c11f
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 10:31:26 2020 +0100

    stat_dev_1:moved all calls to mean functions to loops

commit 922e523
Author: Vandenplas, Jeremie <[email protected]>
Date:   Tue Jan 21 09:13:10 2020 +0100

    stat_dev_1: update test_mean

commit 5a1adcb
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 23:55:15 2020 +0100

    stat_dev: inverting loops for efficiency

commit 86970ae
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 22:54:55 2020 +0100

    stat_dev: use specific interface

commit 6574a67
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 22:36:33 2020 +0100

    stat_dev: addition of calls to error_stop

commit e98090b
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 21:32:27 2020 +0100

    stat_dev: extension to rank 15

commit e0e3092
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 12:46:44 2020 +0100

    stat_dev: simplified merge

commit 22ff6e4
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 10:38:41 2020 +0100

    stat_dev: progress rank 3

commit 7612613
Author: Vandenplas, Jeremie <[email protected]>
Date:   Mon Jan 20 10:34:06 2020 +0100

    stat_dev: add rank 3

commit 60ab523
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 22:14:51 2020 +0100

    stat_dev: addition of integer cases

commit 6fb6ca5
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 21:03:46 2020 +0100

    stat_dev: avoid allocatable functions

commit a1c6353
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 20:11:49 2020 +0100

    modification to have the same behaviour as Fortran sum

commit 72500e1
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 15:23:34 2020 +0100

    stat_dev: add error_stop

commit 1272574
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 15:02:52 2020 +0100

    stat_dev: update Makefile

commit 426d43f
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 12:21:09 2020 +0100

    stat_dev: addition of test and creation of modules and submodules with fypp

    how to use pure functions inside submodules

commit 965f37b
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 11:35:11 2020 +0100

    moved to submodules

    how to use pure functions in submodules

commit d9af336
Author: Vandenplas, Jeremie <[email protected]>
Date:   Sun Jan 19 11:22:52 2020 +0100

    stat_dev: init

commit dc7e49b
Merge: f300f4a cb7cf71
Author: Ondřej Čertík <[email protected]>
Date:   Tue Jan 14 11:11:12 2020 -0700

    Merge pull request fortran-lang#109 from nncarlson/target-include

    Update CMakeLists handling of .mod files

commit cb7cf71
Author: Neil Carlson <[email protected]>
Date:   Mon Jan 13 16:38:12 2020 -0700

    Update CMakeLists handling of .mod files

commit f300f4a
Merge: 12bd060 0ea0ee1
Author: Milan Curcic <[email protected]>
Date:   Wed Jan 8 18:20:09 2020 -0500

    Merge pull request fortran-lang#54 from scivision/systemlib

    add system module

commit 0ea0ee1
Author: Michael Hirsch, Ph.D <[email protected]>
Date:   Mon Jan 6 11:41:46 2020 -0500

    make sleep test automated check with system_clock

commit c8974dc
Author: Michael Hirsch, Ph.D <[email protected]>
Date:   Mon Dec 30 16:20:55 2019 -0500

    add system module

    There are a number of capabilities it would be useful to bring from
    cstdlib and STL. This is an initial demonstration, replacing
    the non-cross-compiler sleep() with a standard implmeentation that
    works across compilers and operating systems, with millisecond integer
    input.

commit 12bd060
Merge: 006beda e1d861d
Author: Ondřej Čertík <[email protected]>
Date:   Wed Jan 8 11:52:06 2020 -0700

    Merge pull request fortran-lang#97 from certik/goals

    Add Goals and Motivation section into README

commit e1d861d
Author: Ondřej Čertík <[email protected]>
Date:   Wed Jan 8 10:00:27 2020 -0700

    Update README.md

commit ca4554a
Author: Ondřej Čertík <[email protected]>
Date:   Wed Jan 8 09:49:39 2020 -0700

    Add Goals and Motivation section into README

commit 006beda
Merge: 1926ade e81d295
Author: Ondřej Čertík <[email protected]>
Date:   Tue Jan 7 15:41:39 2020 -0700

    Merge pull request fortran-lang#94 from certik/workflow

    Document workflow based on the discussion in #5

commit e81d295
Author: Ondřej Čertík <[email protected]>
Date:   Tue Jan 7 08:20:24 2020 -0700

    Update the workflow based on feedback

commit 1926ade
Merge: 7a6108e f857482
Author: Ondřej Čertík <[email protected]>
Date:   Tue Jan 7 08:01:39 2020 -0700

    Merge pull request fortran-lang#96 from nshaffer/dev-optval

    Make optval pure or pure elemental where possible

commit f857482
Merge: 274a2bb 7a6108e
Author: Ondřej Čertík <[email protected]>
Date:   Tue Jan 7 07:48:17 2020 -0700

    Merge branch 'master' into dev-optval

commit 274a2bb
Author: Nathaniel Shaffer <[email protected]>
Date:   Tue Jan 7 07:00:21 2020 -0700

    add tests for 1d arrays (reals, ints, logical)

commit e06e322
Author: Nathaniel Shaffer <[email protected]>
Date:   Tue Jan 7 06:58:14 2020 -0700

    add "elemental" and/or "pure" attributes where possible

commit 92926e0
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 15:01:31 2020 -0700

    Make the specification requirement part of step 3.

commit 1f56d0d
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 12:09:48 2020 -0700

    Document workflow based on the discussion in #5

commit 7a6108e
Merge: e2b0cda a606606
Author: Izaak "Zaak" Beekman <[email protected]>
Date:   Mon Jan 6 13:02:55 2020 -0500

    ci ctest enhancements (fortran-lang#92)

    Merge [scivision:citime] into master

    [scivision:citime]: https://github.com/scivision/stdlib/tree/citime

commit a606606
Author: Michael Hirsch, Ph.D <[email protected]>
Date:   Mon Jan 6 11:50:17 2020 -0500

    ci ctest enhancements

commit e2b0cda
Merge: 57d99f8 f0a6886
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 08:01:24 2020 -0700

    Merge pull request fortran-lang#90 from certik/stream

    Use access = "stream" by default

commit f0a6886
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 07:56:18 2020 -0700

    Update src/stdlib_experimental_io.f90

    Co-Authored-By: Jeremie Vandenplas <[email protected]>

commit 05540fd
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 07:52:24 2020 -0700

    Use access = "stream" by default

commit 57d99f8
Merge: c3e4816 d845f2d
Author: Ondřej Čertík <[email protected]>
Date:   Mon Jan 6 07:42:42 2020 -0700

    Merge pull request fortran-lang#89 from pdebuyl/qsavetxt_format_string

    Use explicit formatting in qsavetxt

commit d845f2d
Author: Pierre de Buyl <[email protected]>
Date:   Mon Jan 6 10:35:55 2020 +0100

    Use explicit formatting in qsavetxt
@urbanjost
Copy link

urbanjost commented Jan 27, 2020

Are we discussing something like an interface to C system routines like often proposed for POSIX interfaces? I use routines like that a lot but cooked my own (not sure how many platforms they would work on, but worked on all the ones I needed) in
M_system. A lot of compilers supported some variant of a "POSIX" interface, but in different ways, which made it highly non-portable. Never quite sure why this did not become part of the standard, especially when "POSIX" was a hot topic. The only one I remember doing a full PXF interface was Cray.

PS: FUNIX shows a lot of simple examples that use the interface. Shows how just the addition of a few things like those being proposed in stdlib make Fortran just as good if not better than most languages for creating basic utilities.

@nncarlson
Copy link
Contributor

The NAG compiler comes with a collection of intrinsic f90_unix_* modules that implement a bunch of POSIX stuff. They should be examples to consider if we wanted to head in that direction with stdlib. The documentation for them is here.

@urbanjost
Copy link

Almost the same list as I have; which is a good sign it can be done relatively generically. Cray, Intel, and gfortran all have a lot of the same functionality as extensions, but all as proprietary extensions so not very portable. Probably others too. None are OOPs, which is interesting.

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.

6 participants