search

UKZN-Astronomy / corrcal

Python/C code for calibration of quasi-redundant arrays. Different (fixed) algorithm from original corrcal
BSD 2-Clause "Simplified" License
1 stars 0 forks source link

Should C codes be updated to newer standard? #19

Open piyanatk opened 4 years ago

piyanatk commented 4 years ago

Parts of the C codes use C99 standard, which require -std=c99 flag to compile. Should we update to newer standard?

@steven-murray What is your opinion?

steven-murray commented 4 years ago

That sounds reasonable to me. Do you know which parts have this behaviour? Are they easy to update?

piyanatk commented 4 years ago

It is probably straightforward if you are familiar with C. One C99 specific syntax is the declaration and assignment of a variable inside the for loop.

for (int targ=0;targ<n;targ++) {
...
steven-murray commented 4 years ago

Well, that's easily fixable!

piyanatk commented 4 years ago

I will give it a shot.

piyanatk commented 4 years ago

So it turned out that the syntax above has been supported since GCC 4.5, meaning that -std=c99 flag should not be required since GCC 4.5+. I double check by removing the flag and compiling with GCC 7, 8, and 9 from homebrew, and indeed the code compiled fine. I am not sure what caused the issue before.

Should I go ahead with the removal of the -std=c99 flag and indicate a GCC version requirement in the README?

piyanatk commented 4 years ago

@steven-murray Can you check if you can still compile https://github.com/UKZN-Astronomy/corrcal/commit/19e46d582e72edaaeba847725154458cdb69f0e3? Also, post your gcc version please?

steven-murray commented 4 years ago

Compiles fine for me (version 9.2.0). What is the default version on OSX and Ubuntu these days?

piyanatk commented 4 years ago

Ubuntu 18.04 has GNU gcc version 7.4 by default. On macOS, you will probably want to install GNU gcc with homebrew, and you can chose the version or install multiple versions. The default gcc on macOS is based on clang which does not support openmp and can be finicky sometimes, so should be avoided. I should probably add these info to README.