Interest in build system upgrades and DevOps integrations #7
Comments
|
Thanks very much - I've forwarded your interesting message to Ryan Maguire,
who is managing the code. I'd be interested in staying in touch about your
use of the code and any issues you have found with the actual science
processing.
Dick French
…On Sat, Oct 8, 2022 at 3:07 PM tcumby ***@***.***> wrote:
On my fork of this repo
<https://github.com/tcumby/rss_ringoccs/tree/incorporate-cmake-into-packagin>
I've been working on converting this project over to a CMake build system
<https://cmake.org/> as well as replacing the python packaging with
scikit-build <https://scikit-build.readthedocs.io/en/latest/>, which was
created to drive a CMake build system as part as packaging a python *.whl.
At this stage, if some user wants to install the rss_ringoccs python
package from my fork the user only needs to run
cd rss_ringoccs/
pip install .
and the required build and installed dependencies will be installed
(including cmake) and then CMake will be executed to build the various
libraries as part of the python packaging.
With this python packaging, I was easily able to add Github CI
integrations, namely a CMake build automation
<https://github.com/tcumby/rss_ringoccs/actions/workflows/cmake.yml> to
test builds on Linux, macOS and Windows (I added Windows build support as
I'm on Windows) in addition to python packaging (on Python 3.6 - 3.9). This Github
CI python action
<https://github.com/tcumby/rss_ringoccs/actions/workflows/python-package.yml>
also executes a number of pytest tests (I converted a number of smoke tests
written in python to pytest) so that when ever a git push occurs to origin,
these tests are executed as part of the Github CI python action. In
essence, whenever a developer pushes code changes they are built an tested
in a clean environment with any issues reported back to the developer (via
email and notifications in github).
I'm curious if there is any interest in incorporating these changes via a
pull request?
p.d. I added Windows/MSVC support following the approach of libcerf,
namely avoid Microsoft's non-standard C complex libraries and instead use
the C++ std::complex implementation, which is standard.
—
Reply to this email directly, view it on GitHub
<#7>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPTRBVV23WF5AOAVNSLILTWCHA5HANCNFSM6AAAAAARALU3FE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
--
Richard G. French
McDowell and Whiting Professor of Astrophysics, Emeritus
Wellesley College
Wellesley, MA 02481-8203
|
|
The latest branch of rss_ringoccs is switching over almost exclusively to C.
A side project of mine has a full implementation of libm with C99 features
including complex numbers and various special functions, but uses only
C89 language (it uses the intersection of C89/C99/C11/C18, so it is very
portable). For compilers supporting IEEE-754/854 floating point arithmetic
and type punning, which almost all do (GCC, LLVM's Clang, TCC, PCC, MSVC, etc.),
this is as fast, and in many cases faster, than other libm implementations.
rss_ringoccs is using this library to make use of complex numbers in a portable fashion.
The result is the next release of rss_ringoccs will be supported on every major operating
system (GNU, Linux, macOS, FreeBSD, Windows, etc.) and several different architectures
(x64, x86, arm64, mips, powerpc, sparc, etc.).
I'm not a big fan of CMake, probably out of reluctance to learn it.
For Unix users there's a Makefile, and for windows there's a batch script.
This is not published yet, but will be with the next release.
Also, I'd like to avoid introducing C++ to keep things simple.
If, after the next release of rss_ringoccs, things are not smooth for Windows users,
I'll definitely look into this as a pull request. I don't anticipate this being the case.
Best,
Ryan
On Sat, 2022-10-08 at 16:17 -0700, RichardGFrench wrote:
Thanks very much - I've forwarded your interesting message to Ryan Maguire,
who is managing the code. I'd be interested in staying in touch about your
use of the code and any issues you have found with the actual science
processing.
Dick French
…On Sat, Oct 8, 2022 at 3:07 PM tcumby ***@***.***> wrote:
On my fork of this repo
<https://github.com/tcumby/rss_ringoccs/tree/incorporate-cmake-into-packagin>
I've been working on converting this project over to a CMake build system
<https://cmake.org/> as well as replacing the python packaging with
scikit-build <https://scikit-build.readthedocs.io/en/latest/>, which was
created to drive a CMake build system as part as packaging a python *.whl.
At this stage, if some user wants to install the rss_ringoccs python
package from my fork the user only needs to run
cd rss_ringoccs/
pip install .
and the required build and installed dependencies will be installed
(including cmake) and then CMake will be executed to build the various
libraries as part of the python packaging.
With this python packaging, I was easily able to add Github CI
integrations, namely a CMake build automation
<https://github.com/tcumby/rss_ringoccs/actions/workflows/cmake.yml> to
test builds on Linux, macOS and Windows (I added Windows build support as
I'm on Windows) in addition to python packaging (on Python 3.6 - 3.9). This Github
CI python action
<https://github.com/tcumby/rss_ringoccs/actions/workflows/python-package.yml>
also executes a number of pytest tests (I converted a number of smoke tests
written in python to pytest) so that when ever a git push occurs to origin,
these tests are executed as part of the Github CI python action. In
essence, whenever a developer pushes code changes they are built an tested
in a clean environment with any issues reported back to the developer (via
email and notifications in github).
I'm curious if there is any interest in incorporating these changes via a
pull request?
p.d. I added Windows/MSVC support following the approach of libcerf,
namely avoid Microsoft's non-standard C complex libraries and instead use
the C++ std::complex implementation, which is standard.
—
Reply to this email directly, view it on GitHub
<#7>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPTRBVV23WF5AOAVNSLILTWCHA5HANCNFSM6AAAAAARALU3FE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Actually, now that I think of it, the one thing I've neglected is the scripts for
downloading kernel files and various data.
For unix users we use a bash script that just runs curl or wget.
I've not looked into the Windows counterpart for this.
Does your CMake file handle this?
…-Ryan
On Sun, 2022-10-09 at 09:43 -0400, ***@***.*** wrote:
The latest branch of rss_ringoccs is switching over almost exclusively to C.
A side project of mine has a full implementation of libm with C99 features
including complex numbers and various special functions, but uses only
C89 language (it uses the intersection of C89/C99/C11/C18, so it is very
portable). For compilers supporting IEEE-754/854 floating point arithmetic
and type punning, which almost all do (GCC, LLVM's Clang, TCC, PCC, MSVC, etc.),
this is as fast, and in many cases faster, than other libm implementations.
rss_ringoccs is using this library to make use of complex numbers in a portable fashion.
The result is the next release of rss_ringoccs will be supported on every major operating
system (GNU, Linux, macOS, FreeBSD, Windows, etc.) and several different architectures
(x64, x86, arm64, mips, powerpc, sparc, etc.).
I'm not a big fan of CMake, probably out of reluctance to learn it.
For Unix users there's a Makefile, and for windows there's a batch script.
This is not published yet, but will be with the next release.
Also, I'd like to avoid introducing C++ to keep things simple.
If, after the next release of rss_ringoccs, things are not smooth for Windows users,
I'll definitely look into this as a pull request. I don't anticipate this being the case.
Best,
Ryan
On Sat, 2022-10-08 at 16:17 -0700, RichardGFrench wrote:
Thanks very much - I've forwarded your interesting message to Ryan Maguire,
who is managing the code. I'd be interested in staying in touch about your
use of the code and any issues you have found with the actual science
processing.
Dick French
On Sat, Oct 8, 2022 at 3:07 PM tcumby ***@***.***> wrote:
On my fork of this repo
<https://github.com/tcumby/rss_ringoccs/tree/incorporate-cmake-into-packagin>
I've been working on converting this project over to a CMake build system
<https://cmake.org/> as well as replacing the python packaging with
scikit-build <https://scikit-build.readthedocs.io/en/latest/>, which was
created to drive a CMake build system as part as packaging a python *.whl.
At this stage, if some user wants to install the rss_ringoccs python
package from my fork the user only needs to run
cd rss_ringoccs/
pip install .
and the required build and installed dependencies will be installed
(including cmake) and then CMake will be executed to build the various
libraries as part of the python packaging.
With this python packaging, I was easily able to add Github CI
integrations, namely a CMake build automation
<https://github.com/tcumby/rss_ringoccs/actions/workflows/cmake.yml> to
test builds on Linux, macOS and Windows (I added Windows build support as
I'm on Windows) in addition to python packaging (on Python 3.6 - 3.9). This Github
CI python action
<https://github.com/tcumby/rss_ringoccs/actions/workflows/python-package.yml>
also executes a number of pytest tests (I converted a number of smoke tests
written in python to pytest) so that when ever a git push occurs to origin,
these tests are executed as part of the Github CI python action. In
essence, whenever a developer pushes code changes they are built an tested
in a clean environment with any issues reported back to the developer (via
email and notifications in github).
I'm curious if there is any interest in incorporating these changes via a
pull request?
p.d. I added Windows/MSVC support following the approach of libcerf,
namely avoid Microsoft's non-standard C complex libraries and instead use
the C++ std::complex implementation, which is standard.
—
Reply to this email directly, view it on GitHub
<#7>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPTRBVV23WF5AOAVNSLILTWCHA5HANCNFSM6AAAAAARALU3FE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Hi Ryan, Thanks for the feedback. Your method of supporting cross-platform development sounds far better; I merely went the C++ route after partially working through MSVC's non-standard complex number implementation only to find that libcerf supports only C++ on MSVC. I definitely worried that I'd violated one or more rules and certainly and certainly peppering source with Yes, CMake is a not for everyone, but it really has become the dominate build system generator for C++ development at least. Also, modern CMake (i.e. the new target-based way to defining your build) is far nicer than what it used to be, and allows you to offload a lot of complexity onto the build system. Given how common it has become, it does make the prospect of integrating other libraries/frameworks far easier. Regarding the kernel download scripts, I merely rewrote them in python 3 (example), but they could likely be written in CMake as well. I wasn't entirely sure where they fit into the workflow (nor the frequency of use), so I just translated them to python and added sufficient logging. As these python scripts exist now, they could trivially added as a console script /entry points such that pip installs them to the If the download scripts were folded into the CMake build system (generally one uses FetchContent to do these tasks), I'd define one or more CMake variables that could be set from the command-line before building. It would likely be a little less flexible, in terms of the user-experience, but if the download only needs to happen once then perhaps it's not a big deal. Is there any other aspect that you'd like someone else to look at? Best regards, |
|
Yes, I was very sad at the millions of compiling errors I initially got with
MSVC for using complex.h and other C99 features. The compiler is at
least C89 compliant, as far as I can tell.
Also, in the latest branch we have indeed peppered header files with
#ifdef __cplusplus
extern "C" {
#endif
...
#ifdef __cplusplus
}
#endif
So rss_ringoccs can be run as a C++ library as well.
I'm not sure how well the CPython API works with C++,
and all of the python portion has been rewritten using this.
I had some thought about it, and I would actually like to have CMake available.
Getting GNU's make running on windows is difficult, and CMake seems more
popular than batch files. Having both options available seems like it wouldn't
harm anyone.
Is you fork based on the master branch of rss_ringoccs, or the v1.3-dev branch?
The master branch is quite dated.
When the next release is available the dependencies will be my own C library
for special functions/complex arithmetic/basic optics routines. That is found here:
https://github.com/ryanmaguire/libtmpl
For x64/x86/arm64 machines some assembly code is used to get a speed boost.
For example, sincos is available on those architectures to efficiently compute
cos(t) + i sin(t) for complex numbers, something rss_ringoccs uses a lot in the
diffraction reconstruction process.
Would it be difficult using CMake to exclude the various .c files from being compiled
and instead assemble the .S files on those targets?
The assembly code is written in NASM, which both GCC and Clang can understand.
I am not sure if MSVC can, however.
Using Make, I do something like:
ifeq ($(uname_m),$(filter $(uname_m),x86_64 amd64))
SRCS := $(shell find $(SRC_DIRS) $(EXCLUDE) -not -name "tmpl_sincos_double.c" -and \( -name "*.c" -or -name "*x86_64.S" \))
endif
But that seems unix exclusive since it uses "uname" and "find".
Best,
Ryan
On Sun, 2022-10-09 at 10:18 -0700, tcumby wrote:
Hi Ryan,
Thanks for the feedback. Your method of supporting cross-platform development sounds far better; I merely went the C++ route after partially working through MSVC's non-standard complex number implementation only to find that libcerf supports only C++ on MSVC. I definitely worried that I'd violated one or more rules and certainly and certainly peppering source with #ifdef __cplusplus is no one's idea of pretty code.
Yes, CMake is a not for everyone, but it really has become the dominate build system generator for C++ development at least. Also, modern CMake (i.e. the new target-based way to defining your build) is far nicer than what it used to be, and allows you to offload a lot of complexity onto the build system. Given how common it has become, it does make the prospect of integrating other libraries/frameworks far easier.
Regarding the kernel download scripts, I merely rewrote them in python 3 (example<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftcumby%2Frss_ringoccs%2Fblob%2Fincorporate-cmake-into-packagin%2Fpipeline%2Fget_16kHz_rsr_files_preUSOfailure.py&data=05%7C01%7CRyan.J.Maguire.GR%40dartmouth.edu%7C48e9f589066545e3d02d08daaa1a3fa8%7C995b093648d640e5a31ebf689ec9446f%7C0%7C0%7C638009326993056925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=pYYZ%2BbZyL9eL1w2pW7MjyHwMSWn6KKGRh5zUekWV%2F0k%3D&reserved=0>), but they could likely be written in CMake as well. I wasn't entirely sure where they fit into the workflow (nor the frequency of use), so I just translated them to python and added sufficient logging. As these python scripts exist now, they could trivially added as a console script /entry points<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsetuptools.pypa.io%2Fen%2Flatest%2Fuserguide%2Fentry_point.html&data=05%7C01%7CRyan.J.Maguire.GR%40dartmouth.edu%7C48e9f589066545e3d02d08daaa1a3fa8%7C995b093648d640e5a31ebf689ec9446f%7C0%7C0%7C638009326993056925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WETWKyyoU2rPlNTaimUZlQAy%2F0YA167Z0Yo1Z9To4KU%3D&reserved=0> such that pip installs them to the <python root>/bin folder (pip itself is an example of this).
If the download scripts were folded into the CMake build system (generally one uses FetchContent<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcmake.org%2Fcmake%2Fhelp%2Flatest%2Fmodule%2FFetchContent.html&data=05%7C01%7CRyan.J.Maguire.GR%40dartmouth.edu%7C48e9f589066545e3d02d08daaa1a3fa8%7C995b093648d640e5a31ebf689ec9446f%7C0%7C0%7C638009326993056925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=o6Q2ZdFGHPNobbxU4%2BYLcHy%2FqQoPrvaQkfW4Q8oZeRQ%3D&reserved=0> to do these tasks), I'd define one or more CMake variables that could be set from the command-line before building. It would likely be a little less flexible, in terms of the user-experience, but if the download only needs to happen once then perhaps it's not a big deal.
Is there any other aspect that you'd like someone else to look at?
Best regards,
Ty
—
Reply to this email directly, view it on GitHub<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FNASA-Planetary-Science%2Frss_ringoccs%2Fissues%2F7%23issuecomment-1272588235&data=05%7C01%7CRyan.J.Maguire.GR%40dartmouth.edu%7C48e9f589066545e3d02d08daaa1a3fa8%7C995b093648d640e5a31ebf689ec9446f%7C0%7C0%7C638009326993056925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iUtDpZfJQ9iXEgI4LKRWEw8m1UC8%2BvYEB5w0wmYd8Lk%3D&reserved=0>, or unsubscribe<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEJXMKV7GGLSMBPTM3LJV4TWCL45LANCNFSM6AAAAAARALU3FE&data=05%7C01%7CRyan.J.Maguire.GR%40dartmouth.edu%7C48e9f589066545e3d02d08daaa1a3fa8%7C995b093648d640e5a31ebf689ec9446f%7C0%7C0%7C638009326993056925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=cUkbGaRMNxV6ivCH2XxlA8U1ViRBjNHRtsKDBONUdZo%3D&reserved=0>.
You are receiving this because you commented.Message ID: ***@***.***>
|
|
In my fork I was working off of master. Likely I can still use some of the higher-level CMakeLists.txt files, but obviously I'll need to modify those and add new ones. Regarding conditionally swapping out *.c with asm, yes I think that is possible. From an initial bit of research, CMake treats asm as it would any other language (example). Conditionally adding sources to a target is a simple exercise (Cmake allows you to separately declare a library target and specify its source files (see here were I declare the target for librssringoccs and one of the many examples where I add sources to that target). So, conditionally adding source files based on the compiler, or whether the files are even present, should be doable. There do seem to be some sites (example) discussing NASM with Visual Studio. Likely I would want to add NASM in through a package manager. I'm using vcpkg right now and they only have YASM, but Conan has nasm for macOS, Linux and Windows. I'm only using vcpkg for a few dependencies at the moment (GSL and a C++ unit testing framework), so switching to Conan shouldn't be too drastic of a change. |
|
I've started adding a CMake build system to libtmpl and my fork of the 1.3-dev branch of rss_ringoccs. Right now they're pretty rudimentary and lacking the full interfaces of make.sh. I've add the same github CI action that executes Cmake builds in the Github CI for macOS, Ubuntu and Windows. Github CI's runners (e.g. here's the Ubuntu 22.04 image details) install a couple of gcc /clang as well as XCode and/or MSVC. I'm going to try to expand the github cmake action for libtmpl to span multiple compiler versions and/or other compilers. Also, it might make sense to convert some of the unit tests over to one of the standard frameworks (Google Test, Catch2, ...) so that they can emit output that the Github CI can easily consume as part of the cmake github action. |
|
The build system for libtmpl is mostly in place, but I'm reworking how the source file exclusions work so it is easier to maintain. I still need to set up toolchain files and preset files so as to make cross-compiling and general use easier. Thanks for pointing out the complexities of long doubles. Google Test seemed to have reasonable methods for comparing floating point values, but I will have to see that it abstracts away the implementation details of long double. Also, it only supports clang, gcc and msvc compilers, so I might have to look elsewhere, or find a library that can allow one to generate a JUnit XML file "by hand". |
|
I have the CMake system essentially ready for libtmpl and I'll be issuing a PR tomorrow. FYI MSVC is limited to OpenMP 2.0, which doesn't allow unsigned integers as for loop index variables. |
On my fork of this repo I've been working on converting this project over to a CMake build system as well as replacing the python packaging with scikit-build, which was created to drive a CMake build system as part as packaging a python *.whl. At this stage, if some user wants to install the rss_ringoccs python package from my fork the user only needs to run
cd rss_ringoccs/pip install .and the required build and installed dependencies will be installed (including cmake) and then CMake will be executed to build the various libraries as part of the python packaging.
With this python packaging, I was easily able to add Github CI integrations, namely a CMake build automation to test builds on Linux, macOS and Windows (I added Windows build support as I'm on Windows) in addition to python packaging (on Python 3.6 - 3.9). This Github CI python action also executes a number of pytest tests (I converted a number of smoke tests written in python to pytest) so that when ever a git push occurs to origin, these tests are executed as part of the Github CI python action. In essence, whenever a developer pushes code changes they are built an tested in a clean environment with any issues reported back to the developer (via email and notifications in github).
I'm curious if there is any interest in incorporating these changes via a pull request?
p.d. I added Windows/MSVC support following the approach of libcerf, namely avoid Microsoft's non-standard C complex libraries and instead use the C++ std::complex implementation, which is standard.
The text was updated successfully, but these errors were encountered: