Skip to content

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 <jeremie.vandenplas@gmail.com> 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 <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 20:44:24 2020 +0100 stat_cmake: update Makefile commit 17e3d16 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 20:35:19 2020 +0100 second try cmake commit 397eb18 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 20:18:37 2020 +0100 Modifications of CMake for tests on Ubuntu 7 commit e96c12d Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 19:40:05 2020 +0100 small change in md commit 7eec9ae Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 19:37:06 2020 +0100 stat_dev: renamed stat to stats commit 8199b6d Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 19:26:20 2020 +0100 stat_dev: changed spec commit b1c481d Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 19:15:25 2020 +0100 stat_dev: modifs following comments commit e64657c Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> 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 <jeremie.vandenplas@gmail.com> 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 <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 13:13:00 2020 +0100 stat_dev_1: changed all to iterations commit 8d4c11f Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> 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 <jeremie.vandenplas@gmail.com> Date: Tue Jan 21 09:13:10 2020 +0100 stat_dev_1: update test_mean commit 5a1adcb Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 23:55:15 2020 +0100 stat_dev: inverting loops for efficiency commit 86970ae Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 22:54:55 2020 +0100 stat_dev: use specific interface commit 6574a67 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 22:36:33 2020 +0100 stat_dev: addition of calls to error_stop commit e98090b Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 21:32:27 2020 +0100 stat_dev: extension to rank 15 commit e0e3092 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 12:46:44 2020 +0100 stat_dev: simplified merge commit 22ff6e4 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 10:38:41 2020 +0100 stat_dev: progress rank 3 commit 7612613 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Mon Jan 20 10:34:06 2020 +0100 stat_dev: add rank 3 commit 60ab523 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 22:14:51 2020 +0100 stat_dev: addition of integer cases commit 6fb6ca5 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 21:03:46 2020 +0100 stat_dev: avoid allocatable functions commit a1c6353 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 20:11:49 2020 +0100 modification to have the same behaviour as Fortran sum commit 72500e1 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 15:23:34 2020 +0100 stat_dev: add error_stop commit 1272574 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 15:02:52 2020 +0100 stat_dev: update Makefile commit 426d43f Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> 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 <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 11:35:11 2020 +0100 moved to submodules how to use pure functions in submodules commit d9af336 Author: Vandenplas, Jeremie <jeremie.vandenplas@gmail.com> Date: Sun Jan 19 11:22:52 2020 +0100 stat_dev: init commit dc7e49b Merge: f300f4a cb7cf71 Author: Ondřej Čertík <ondrej@certik.us> 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 <neil.n.carlson@gmail.com> Date: Mon Jan 13 16:38:12 2020 -0700 Update CMakeLists handling of .mod files commit f300f4a Merge: 12bd060 0ea0ee1 Author: Milan Curcic <caomaco@gmail.com> 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 <scivision@users.noreply.github.com> Date: Mon Jan 6 11:41:46 2020 -0500 make sleep test automated check with system_clock commit c8974dc Author: Michael Hirsch, Ph.D <scivision@users.noreply.github.com> 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 <ondrej@certik.us> 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 <ondrej@certik.us> Date: Wed Jan 8 10:00:27 2020 -0700 Update README.md commit ca4554a Author: Ondřej Čertík <ondrej@certik.us> 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 <ondrej@certik.us> 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 <ondrej@certik.us> 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 <ondrej@certik.us> 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 <ondrej@certik.us> Date: Tue Jan 7 07:48:17 2020 -0700 Merge branch 'master' into dev-optval commit 274a2bb Author: Nathaniel Shaffer <nrshaffer@protonmail.com> Date: Tue Jan 7 07:00:21 2020 -0700 add tests for 1d arrays (reals, ints, logical) commit e06e322 Author: Nathaniel Shaffer <nrshaffer@protonmail.com> Date: Tue Jan 7 06:58:14 2020 -0700 add "elemental" and/or "pure" attributes where possible commit 92926e0 Author: Ondřej Čertík <ondrej@certik.us> Date: Mon Jan 6 15:01:31 2020 -0700 Make the specification requirement part of step 3. commit 1f56d0d Author: Ondřej Čertík <ondrej@certik.us> 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 <zbeekman@gmail.com> 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 <scivision@users.noreply.github.com> Date: Mon Jan 6 11:50:17 2020 -0500 ci ctest enhancements commit e2b0cda Merge: 57d99f8 f0a6886 Author: Ondřej Čertík <ondrej@certik.us> 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 <ondrej@certik.us> Date: Mon Jan 6 07:56:18 2020 -0700 Update src/stdlib_experimental_io.f90 Co-Authored-By: Jeremie Vandenplas <jeremie.vandenplas@gmail.com> commit 05540fd Author: Ondřej Čertík <ondrej@certik.us> Date: Mon Jan 6 07:52:24 2020 -0700 Use access = "stream" by default commit 57d99f8 Merge: c3e4816 d845f2d Author: Ondřej Čertík <ondrej@certik.us> 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 <pdebuyl@pdebuyl.be> 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

6 participants