[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[gnuastro-commits] master 00d829a 1/2: Library (txt.c): stdintimeout val
|
From: |
Mohammad Akhlaghi |
|
Subject: |
[gnuastro-commits] master 00d829a 1/2: Library (txt.c): stdintimeout value passed as seconds+microseconds |
|
Date: |
Sat, 10 Jul 2021 12:29:20 -0400 (EDT) |
branch: master
commit 00d829a4445467ec2ea5927af216c9bd4d805fce
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Library (txt.c): stdintimeout value passed as seconds+microseconds
Until now, the value given to 'stdintimeout' was directly passed onto the
'tv_usec' variable (that stores the delay time in micro-seconds) and the
value of 'tv_sec' (that stores the delay time in seconds) was set to
zero. This was based on a wrong understanding of these values! I mistakenly
thought that only one of the two should be used. However, they are
complementary: the number of seconds to delay should be given to 'tv_sec'
and the number of extra micro-seconds should be given to 'tv_usec'. In
other words, 'tv_usec' should not be larger than 1000000 (microseconds). As
a result, increasing the value of '--stdintimeout' beyond one million was
not effective.
On the other hand, the default waiting time for input from standard input
(a pipe) was 0.1 seconds (or 100000 micro-seconds). This could cause
confusing error messages for some users when the previous command takes
long. Thus forcing the user to increase the value with the '--stdintimeout'
option manually.
In line with the issue above, Zahra Sharbaf reported a bug where the test
for the sort-by-night script during 'make check' failed, because of this
error. Unfortunately a normal user had no way of fixing this without
manually editing the sort-by-night script (which is not good).
With this commit, the issues above have been addressed: 'tv_sec' and
'tv_usec' are now properly set, the default value of --stdintimeout has
been increased to 1.5 seconds (or 1500000 microseconds). This is still
enough time for a normal user to see an error if there is a problem in the
previous command (its not too long). But will greatly reduce the number of
un-expected errors. Also, generally, all programs are set to avoid checking
for standard input when a file has already been given (only Convolve was
not using the 'gal_options_check_stdin' function which does this
automatically, but it now uses it). For ConvertType, since the first
channel may be from the standard input, its '--stdintimeout' has been set
to the old value of 0.1sec.
Finally, since the sort-by-night script may be given thousands of images
that may take even longer than the default '--stdintimeout' value, this
option has been added to this script and passed to all the programs called
by the script that need it.
This commit fixes bug #60901 that was reported by Zahra Sharbaf.
---
Makefile.am | 2 +-
NEWS | 15 +++++++++++
bin/convertt/astconvertt.conf | 5 ++++
bin/convertt/ui.c | 16 ++++++++----
bin/convolve/ui.c | 19 +++++++++-----
bin/gnuastro.conf | 2 +-
bin/script/sort-by-night.in | 22 +++++++++++++---
bin/statistics/ui.c | 3 ++-
bootstrap.conf | 1 +
doc/gnuastro.texi | 7 +++++
lib/txt.c | 43 ++++++++++++++++++++++---------
tests/Makefile.am | 27 ++++++++++++++-----
tests/convolve/spectrum-1d.sh | 60 +++++++++++++++++++++++++++++++++++++++++++
13 files changed, 184 insertions(+), 38 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 113fb23..0e38d42 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -336,7 +336,7 @@ bin/completion.bash: $(top_srcdir)/bin/completion.bash.in
## the message (making the message hard to notice for a user).
all-local: bin/completion.bash
- # If we are in static linking mode, correct the 'lib_dependencies'
+ # If we are in static linking mode, correct the 'dependency_libs'
# variable of 'libgnuastro.la'. Because by default it will not
# include static libraries!
@if [ "X$(MAKECMDGOALS)" = "Xall-am" ] && [ "X$(ENABLE_SHARED)" = "Xno"
]; then \
diff --git a/NEWS b/NEWS
index 6c7972e..a1475f0 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,11 @@ See the end of the file for license conditions.
repetition. See description of '--outcols' in the Match manual for
more. This feature was proposed by Juan Molina Tobar and Leslie Hunt.
+ Sort-by-night (installed script)
+ --stdintimeout: new option to set internal delay time for input from
+ pipes (standard input). This is not for inputs to the script (which
+ should always be files), but for the script's internal pipes.
+
Library:
- Arithmetic macros:
- GAL_ARITHMETIC_OP_BOX_AROUND_ELLIPSE
@@ -37,6 +42,11 @@ See the end of the file for license conditions.
** Changed features
+ All programs:
+ --stdintimeout: default value changed from 0.1 seconds to 1.5 seconds to
+ avoid too many crashes when command before the pipe takes longer (thus
+ needing a manual change of this value).
+
Match:
- The two '--notmatched' and '--outcols' can be called together (to
create a single catalog that appends the non-matching rows of second
@@ -51,6 +61,11 @@ See the end of the file for license conditions.
this bug was reported by Zahra Sharbaf.
bug #60826: Arithmetic won't delete existing file with tofile operators.
bug #60881: Query segmentation fault when NED is called without a dataset.
+ bug #60901: sort-by-night crash during make check due to stdintimeout,
+ this bug was reported by Zahra Sharbaf.
+ bug #60903: Increasing value of stdintimeout not effective beyond 1000000,
+ this bug was reported by Zahra Sharbaf.
+
diff --git a/bin/convertt/astconvertt.conf b/bin/convertt/astconvertt.conf
index be36603..2ba78e4 100644
--- a/bin/convertt/astconvertt.conf
+++ b/bin/convertt/astconvertt.conf
@@ -32,3 +32,8 @@
invert 0
# Common options
+#
+# --stdintimeout: If a standard input is given, ConvertType will use it as
+# the first color channel. So we need to actually check it all the time and
+# can't use the common value for all of Gnuastro (since it will take long).
+ stdintimeout 10000
diff --git a/bin/convertt/ui.c b/bin/convertt/ui.c
index 8be4123..20c35cf 100644
--- a/bin/convertt/ui.c
+++ b/bin/convertt/ui.c
@@ -738,11 +738,17 @@ ui_prepare_input_channels(struct converttparams *p)
/* Make sure there are 1 (for grayscale), 3 (for RGB) or 4 (for CMYK)
color channels. */
if(p->numch!=1 && p->numch!=3 && p->numch!=4)
- error(EXIT_FAILURE, 0, "the number of input color channels has to be "
- "1 (for non image data, grayscale or only K channel in CMYK), "
- "3 (for RGB) and 4 (for CMYK). You have given %zu color channels. "
- "Note that some file formats (for example JPEG in RGB mode) can "
- "contain more than one color channel", p->numch);
+ error(EXIT_FAILURE, 0, "the number of input color channels has to "
+ "be 1 (for non image data, grayscale or only K channel in "
+ "CMYK), 3 (for RGB) and 4 (for CMYK). You have given %zu "
+ "color channels. Note 1: some file formats (for example "
+ "JPEG in RGB mode) can contain more than one color channel, "
+ "if such a file is given all its channels are read, so "
+ "separate them first. Note 2: if your first input channel "
+ "was given through the standard input (piped from another "
+ "program) you can fix this error by giving a larger value "
+ "to the '--stdintimeout' option (currently %ld "
+ "micro-seconds)", p->numch, p->cp.stdintimeout);
/* If there are multiple color channels, then ignore the monotocolor
diff --git a/bin/convolve/ui.c b/bin/convolve/ui.c
index 99367da..90bdf95 100644
--- a/bin/convolve/ui.c
+++ b/bin/convolve/ui.c
@@ -326,14 +326,16 @@ ui_read_column(struct convolveparams *p, int i0k1)
int tformat;
char *source;
gal_data_t *out, *cinfo;
- gal_list_str_t *column=NULL;
+ gal_list_str_t *lines, *column=NULL;
size_t ncols, nrows, counter=0;
char *hdu = i0k1==0 ? p->cp.hdu : p->khdu;
char *name = i0k1==0 ? "input" : "kernel";
char *filename = i0k1==0 ? p->filename : p->kernelname;
char *columnname = i0k1==0 ? p->column : p->kernelcolumn;
- gal_list_str_t *lines = gal_options_check_stdin(filename,
- p->cp.stdintimeout, name);
+
+ /* Read input from standard input when necessary (only if 'filename' is
+ not given). */
+ lines=gal_options_check_stdin(filename, p->cp.stdintimeout, name);
/* If no column is specified, Convolve will abort and an error will be
printed when the table has more than one column. If there is only one
@@ -370,9 +372,8 @@ ui_read_column(struct convolveparams *p, int i0k1)
" $ info gnuastro \"Selecting table columns\"\n",
( filename
? gal_checkset_dataset_name(filename, hdu)
- : "Standard input" ));
+ : "standard input (pipe from another program)" ));
}
-
}
gal_list_str_add(&column, columnname, 0);
@@ -382,8 +383,8 @@ ui_read_column(struct convolveparams *p, int i0k1)
NULL);
gal_list_str_free(lines, 1);
- /* Confirm if only one column was read (it is possible to match more than
- one column). */
+ /* Confirm if only one column was read (it is possible that a regexp
+ '--searchin' value, gives a match to more than one column). */
if(out->next!=NULL)
{
if(filename)
@@ -415,6 +416,10 @@ ui_read_column(struct convolveparams *p, int i0k1)
gal_checkset_allocate_copy("standard-input",
( i0k1==0 ? &p->filename : &p->kernelname ));
+ /* One-dimensional convolution is only supported in spatial-mode
+ convolution. */
+ p->domain=CONVOLVE_DOMAIN_SPATIAL;
+
/* Clean up and return. */
gal_list_str_free(column, 0);
return out;
diff --git a/bin/gnuastro.conf b/bin/gnuastro.conf
index a93d7cb..cf13205 100644
--- a/bin/gnuastro.conf
+++ b/bin/gnuastro.conf
@@ -23,7 +23,7 @@
hdu 1
ignorecase 1
searchin name
- stdintimeout 100000
+ stdintimeout 1500000
# Tessellation
tilesize 30,30
diff --git a/bin/script/sort-by-night.in b/bin/script/sort-by-night.in
index 37f82c6..3fa3022 100644
--- a/bin/script/sort-by-night.in
+++ b/bin/script/sort-by-night.in
@@ -39,6 +39,7 @@ quiet=0
key=DATE
prefix=./
version=@VERSION@
+stdintime=10000000
scriptname=@SCRIPT_NAME@
@@ -98,6 +99,8 @@ $scriptname options:
--cite BibTeX citation for this program.
-q, --quiet Don't print the list.
-V, --version Print program version.
+ --stdintimeout Micro-seconds to wait for standard input
+ (used for operations within this script).
Mandatory or optional arguments to long options are also mandatory or optional
for any corresponding short options.
@@ -198,6 +201,10 @@ do
-p=*|--prefix=*) prefix="${1#*=}"; check_v "$1"
"$prefix"; shift;;
-p*) prefix=$(echo "$1" | sed -e's/-p//'); check_v "$1"
"$prefix"; shift;;
+ # Operating mode options
+ --stdintimeout) stdintime="$2"; check_v "$1"
"$stdintime"; shift;shift;;
+ --stdintimeout=*) stdintime="${1#*=}"; check_v "$1"
"$stdintime"; shift;;
+
# Non-operating options.
-q|--quiet) quiet=1; shift;;
-q*|--quiet=*) on_off_option_error --quiet -q;;
@@ -263,8 +270,9 @@ list=$(astfits --keyvalue=$key --hdu=$hdu $inputs
--colinfoinstdout \
day 1 - \
where' \
--colmetadata=ARITH_8,NIGHT,counter,"Observing
night." \
- --colinfoinstdout \
- | asttable --sort=UNIXSEC --colinfoinstdout)
+ --colinfoinstdout --stdintimeout=$stdintime \
+ | asttable --sort=UNIXSEC --colinfoinstdout \
+ --stdintimeout=$stdintime)
@@ -287,12 +295,17 @@ list=$(astfits --keyvalue=$key --hdu=$hdu $inputs
--colinfoinstdout \
#
# We are using 'asttable' here to avoid issues with spaces in
# directory names in the first line.
-unique=$(echo "$list" | asttable -cNIGHT | sort | uniq | cat -n)
+unique=$(echo "$list" \
+ | asttable --stdintimeout=$stdintime -cNIGHT \
+ | sort \
+ | uniq \
+ | cat -n)
#echo "check: $unique"; exit 1
+
# Find the FITS files of every unique day and sort them by observing
# time within that day. We'll also initialize the night-counter to 1.
echo "$unique" | while read l; do
@@ -301,7 +314,8 @@ echo "$unique" | while read l; do
night_to=$(echo $l | awk '{print $1}')
night_from=$(echo $l | awk '{print $2}')
in_this_night=$(echo "$list" \
- | asttable -cFILENAME --equal=NIGHT,$night_from)
+ | asttable --stdintimeout=$stdintime \
+ -cFILENAME --equal=NIGHT,$night_from)
# Now that we know this night's files, sorted by time, we can take
# the proper action (simply list, or copy or make links).
diff --git a/bin/statistics/ui.c b/bin/statistics/ui.c
index ed6bb38..72944fe 100644
--- a/bin/statistics/ui.c
+++ b/bin/statistics/ui.c
@@ -825,7 +825,8 @@ ui_read_columns(struct statisticsparams *p)
size_t size, ncols, nrows, counter=0;
gal_list_str_t *incols, *columnlist=NULL;
gal_list_str_t *lines=gal_options_check_stdin(p->inputname,
- p->cp.stdintimeout, "input");
+ p->cp.stdintimeout,
+ "input");
/* Merge possibly multiple calls to '-c' (each possibly separated by a
coma) into one list. */
diff --git a/bootstrap.conf b/bootstrap.conf
index e2f2630..9aca00b 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -202,6 +202,7 @@ gnulib_modules="
regex
error
nproc
+ select
stdint
strtod
mktime
diff --git a/doc/gnuastro.texi b/doc/gnuastro.texi
index 0e3525a..cc5cdb1 100644
--- a/doc/gnuastro.texi
+++ b/doc/gnuastro.texi
@@ -21795,6 +21795,13 @@ For example, with @option{--prefix=img-}, all the
created file names in the curr
@option{--prefix} can also be used to store the links/copies in another
directory relative to the directory this script is being run (it must already
exist).
For example @code{--prefix=/path/to/processing/img-} will put all the
links/copies in the @file{/path/to/processing} directory, and the files (in
that directory) will all start with @file{img-}.
+
+@item --stdintimeout=INT
+Number of micro-seconds to wait for standard input within this script.
+This does not correspond to general inputs into the script, inputs to the
script should always be given as a file.
+However, within the script, pipes are often used to pass the output of one
program to another.
+The value given to this option will be passed to those internal pipes.
+When running this script, if you confront an error, saying ``No input!'', you
should be able to fix it by giving a larger number to this option (the default
value is 10000000 micro-seconds or 10 seconds).
@end table
diff --git a/lib/txt.c b/lib/txt.c
index 5ed3978..d7cbaff 100644
--- a/lib/txt.c
+++ b/lib/txt.c
@@ -1206,12 +1206,16 @@ gal_txt_image_read(char *filename, gal_list_str_t
*lines, size_t minmapsize,
static int
txt_stdin_has_contents(long timeout_microsec)
{
+ int sout;
fd_set fds;
struct timeval tv;
- /* Set the timeout time. */
- tv.tv_sec = 0;
- tv.tv_usec = timeout_microsec;
+ /* Set the timeout time. We need to put the number of seconds in 'tv_sec'
+ and the remaining microseconds in 'tv_usec' (this cannot be larger
+ than one million, otherwise 'select' is going to abort with an
+ error). */
+ tv.tv_sec = timeout_microsec/1000000;
+ tv.tv_usec = timeout_microsec%1000000;
/* Initialize 'fd_set'. */
FD_ZERO(&fds);
@@ -1219,14 +1223,29 @@ txt_stdin_has_contents(long timeout_microsec)
/* Set standard input (STDIN_FILENO is 0) as the FD that must be read. */
FD_SET(STDIN_FILENO, &fds);
- /* 'select' takes the last file descriptor value + 1 in the fdset to
- check, the fdset for reads, writes, and errors. We are only passing
- in reads. the last parameter is the timeout. select will return if
- an FD is ready or the timeout has occurred. */
- select(STDIN_FILENO+1, &fds, NULL, NULL, &tv);
-
- // return 0 if STDIN is not ready to be read.
- return FD_ISSET(STDIN_FILENO, &fds);
+ /* From Glibc: "The 'select' function blocks the calling process until
+ there is activity on any of the specified sets of file descriptors, or
+ until the timeout period has expired". 'select' takes the last file
+ descriptor value+1 in the 'FD_SET' above as first argument. If the
+ second (reading), third (writing) and fourth (exception) are not NULL,
+ then it will check for the respective property(s).
+
+ When successful (file descriptor has input for the desired action),
+ 'select' will return 1. When the timeout has been reached, it will
+ return 0 and when there was an error it will return -1. If there is an
+ error, we'll abort the program and ask the user to contact us (its a
+ bug).*/
+ errno=0;
+ sout=select(STDIN_FILENO+1, &fds, NULL, NULL, &tv);
+ if(sout==-1)
+ error(EXIT_FAILURE, errno, "%s: a bug! Please contact us at '%s' "
+ "to fix the problem. The 'select' function has detected an "
+ "error", __func__, PACKAGE_BUGREPORT);
+
+ /* By this point, 'sout' only has a value of 1 (stdin is ready for
+ reading) or 0 (timeout was reached with no change, so it should't be
+ used). So simply return the value of 'sout'. */
+ return sout;
}
@@ -1240,7 +1259,7 @@ gal_txt_stdin_read(long timeout_microsec)
gal_list_str_t *out=NULL;
size_t lineno=0, linelen=10;/* 'linelen' will be increased by 'getline'. */
- /* If there is nothing */
+ /* Only continue if standard input has any contents. */
if( txt_stdin_has_contents(timeout_microsec) )
{
/* Allocate the space necessary to keep a copy of each line as we
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 93b6ce5..261cda7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -85,8 +85,10 @@ if COND_CONVERTT
convertt/fitstopdf.sh: crop/section.sh.log
endif
if COND_CONVOLVE
- MAYBE_CONVOLVE_TESTS = convolve/spatial.sh convolve/frequency.sh
+ MAYBE_CONVOLVE_TESTS = convolve/spatial.sh convolve/frequency.sh \
+ convolve/spectrum-1d.sh
+ convolve/spectrum-1d.sh: prepconf.sh.log
convolve/spatial.sh: mkprof/mosaic1.sh.log
convolve/frequency.sh: mkprof/mosaic1.sh.log
endif
@@ -264,12 +266,23 @@ TESTS = prepconf.sh lib/multithread.sh $(MAYBE_CXX_TESTS)
\
-# Files to distribute along with the tests.
-EXTRA_DIST = $(TESTS) during-dev.sh buildprog/simpleio.c crop/cat.txt \
- mkprof/3d-cat.txt match/positions-1.txt match/positions-2.txt \
- mkprof/mkprofcat1.txt mkprof/ellipticalmasks.txt mkprof/clearcanvas.txt \
- mkprof/mkprofcat2.txt mkprof/mkprofcat3.txt mkprof/mkprofcat4.txt \
- mkprof/radeccat.txt statistics/stdin-input.txt table/table.txt
+# Files to distribute within the tarball (sorted alphabetically).
+EXTRA_DIST = $(TESTS) during-dev.sh \
+ buildprog/simpleio.c \
+ convolve/spectrum.txt \
+ crop/cat.txt \
+ mkprof/3d-cat.txt \
+ match/positions-1.txt \
+ match/positions-2.txt \
+ mkprof/mkprofcat1.txt \
+ mkprof/clearcanvas.txt \
+ mkprof/ellipticalmasks.txt \
+ mkprof/mkprofcat2.txt \
+ mkprof/mkprofcat3.txt \
+ mkprof/mkprofcat4.txt \
+ mkprof/radeccat.txt \
+ statistics/stdin-input.txt \
+ table/table.txt
diff --git a/tests/convolve/spectrum-1d.sh b/tests/convolve/spectrum-1d.sh
new file mode 100755
index 0000000..4881836
--- /dev/null
+++ b/tests/convolve/spectrum-1d.sh
@@ -0,0 +1,60 @@
+# Convolve a 1D spectrum, that will also check reading from standard input
+# for the kernel.
+#
+# See the Tests subsection of the manual for a complete explanation
+# (in the Installing gnuastro section).
+#
+# Original author:
+# Mohammad Akhlaghi <mohammad@akhlaghi.org>
+# Contributing author(s):
+# Copyright (C) 2021, Free Software Foundation, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved. This file is offered as-is,
+# without any warranty.
+
+
+
+
+
+# Preliminaries
+# =============
+#
+# Set the variables (The executable is in the build tree). Do the
+# basic checks to see if the executable is made or if the defaults
+# file exists (basicchecks.sh is in the source tree).
+prog=convolve
+execname=../bin/$prog/ast$prog
+spec=$topsrc/tests/$prog/spectrum.txt
+
+
+
+
+
+# Skip?
+# =====
+#
+# If the dependencies of the test don't exist, then skip it. There are two
+# types of dependencies:
+#
+# - The executable was not made (for example due to a configure option),
+#
+# - The input data was not made (for example the test that created the
+# data file failed).
+if [ ! -f $execname ]; then echo "$execname not created."; exit 77; fi
+if [ ! -f $spec ]; then echo "$spec does not exist."; exit 77; fi
+
+
+
+
+# Actual test script
+# ==================
+#
+# 'check_with_program' can be something like Valgrind or an empty
+# string. Such programs will execute the command if present and help in
+# debugging when the developer doesn't have access to the user's system.
+echo "1 3 10 3 1" \
+ | sed 's/ /\n/g' \
+ | $check_with_program $execname $spec --domain=spatial \
+ --output=convolve_spectrum.txt