Namespacing avoids this issue of conflict. You already have PETSCDIFF for the diff. So I changed MPIEXEC to PETSCMPIEXEC Pushed to scott/test-harness-valgrind-v2 and merged to next. Satish On Wed, 8 Feb 2017, Satish Balay wrote:
The old system was just makefile with runtargets using shell.
So we had: - default 'VAR=foo' in petscvariables (i.e make variable) - and we could change the value on command line: 'make VAR=new'
We avoided 'make -e' early-on [due to buggy make?] and continued to do so - to avoid picking up user env that could have conflicting stuff
With the new system we have 'makefile -> gmakefile -> runex.sh' - and we can run tests at all 3 leves. I think its best if all 3 modes behave the same way (or pretty close).
runex.sh will get default values from petsvariables via ./config/gmakegentest.py - so this case works fine.
With the default usage of 'make' [i.e without -e] - things can become inconsistant between make and the script - when user env variables exist. For ex:
$ cat makefile FOO=default_val_make all: @echo makevar: ${FOO} @echo scriptvar: `./script.sh` $ cat script.sh #!/bin/sh FOO=${FOO:-"default_val_script"} echo $FOO
1. Env variable FOO is not set [and we see the expected behavior. - we would have default_val_make and default_val_script be the same] $ ./script.sh default_val_script $ make makevar: default_val_make scriptvar: default_val_script $
2. With 'FOO' specified to make (via command line) This is fine aswell. (no equivalent stuff for script) $ make FOO=cmd_var makevar: cmd_var scriptvar: cmd_var
3. Use env variable. (env var is ignored by make as expected - but script_var gets changed to make_var? I don't understand this - however it won't affect our usage as both should be same).
But now script.sh is picking up this user env variable - which I don't think we would want to happen.
$ export FOO=env_var $ make makevar: default_val_make scriptvar: default_val_make $ ./script.sh env_var
There is also one more thing to worry about:
the usage is: 'makefile -> gmakefile -> runex.sh' -> "bin/*.sh" and the scripts in bin can rely on env variables [PETSC_ARCH/PETSC_DIR] for defaults. So I fixed petscdiff for defaults but I'm avoiding petscmpiexec and using petsc_mpiexec_valgrind() in petsc_harness.sh instead.
[petscdiff currently lets us override defaults with env variables - perhaps it will also have the same issue as above - so should be fixed - or we add petsc_diff() in petsc_harness.sh.]
Satish
On Wed, 8 Feb 2017, Scott Kruger wrote:
I have no strong objection, but a couple of comments.
Barry said that the old test system could take env variables from the command-line. Didn't the old system pull in petscvariables? If so, how could it be overwritten.
I think only documenting (and yes, I will need to update the documentation when things have settled down) VALGRIND=1 but allow the secret -e method would probably be OK. Users that know about make -e should be smart enough (?) to know the danger.
Scott
On 2/8/17 3:47 PM, Satish Balay wrote:
Scott & Barry,
I'd like to remove the 'env' variable change. We don't want this to be overwritten as it can be fragile [as users env can have incompatible stuff here]
And petscmpiexec relies on env variables - so I'm not using it in the test harness.
So I have a branch scott/test-harness-valgrind-v2 (with commits collapsed/fixed)
So the following would be the way to use it.
make -f gmakefile test VALGRIND=1
Do you have any objections to this?
If we want them to be overwritted - then I guess we'll have to namespace them properly.
Satish
On Wed, 8 Feb 2017, Scott Kruger wrote:
Actually Jed's discussion of using env for DIFF had me consider another solution than the one I had originally came up with. I have both mpiexec and diff using this new method to give what Barry wants more directly: https://bitbucket.org/petsc/petsc/pull-requests/631/
Both of these now work at the makefile level:
make -f gmakefile test VALGRIND=1
MPIEXEC="/sandbox/bsmith/petsc/bin/petscmpiexec -valgrind" make -e -f gmakefile test
Note that I am passing the -e flag to make which has the environment variable override the value in ${PETSC_ARCH}/lib/conf/petscvariables This can be dangerous depending on how clean your environment is because it'll override ALL variables.
And from the shell script level:
runex1.sh -V
MPIEXEC="/sandbox/bsmith/petsc/bin/petscmpiexec -valgrind" runex1.sh
On 2/7/17 4:42 PM, Barry Smith wrote:
On Feb 7, 2017, at 5:34 PM, Satish Balay <[email protected]> wrote:
We cannot override stuff via env variables in the new test harness. For nightlybuilds I added support to check if PETSC_ARCH has 'valgrind' string in it - and if so - use valgrind.
26646c0bcc29652ef239540b066d00d1c8fc99c7
Perhaps we can force this to use valgrind with:
./config/gmakegentest.py --valgrind=1
I can't decide at the time I run the tests, but need to decide when I do the gmakegentest.py
Ok - perhaps the attached patch.
That is one heck of a patch, talk about code duplication resulting in endless bugs.
But then - sometimes gmakefile runs ./config/gmakegentest.py directly [without valgrind]
Fix it properly somehow; currently it is impossible and that is insanely horrible.
Satish
-------
On Tue, 7 Feb 2017, Barry Smith wrote:
Both
make -f gmakefile test MPIEXEC="/sandbox/bsmith/petsc/bin/petscmpiexec -valgrind "
and
MPIEXEC="/sandbox/bsmith/petsc/bin/petscmpiexec -valgrind " make -f gmakefile test
do not appear to use valgrind in the test harness (the old system did).
How do I run all tests under valgrind automatically?
not ok dm_impls_plex_tests-ex18_1 # [0]PETSC ERROR: PetscTrFreeDefault() called from VecDestroy_MPI() line 18 in /sandbox/bsmith/petsc/src/vec\ /vec/impls/mpi/pdvec.c # [0]PETSC ERROR: Block [id=0(32)] at address 0xb54240 is # corrupted (probably write past end of array) # [0]PETSC ERROR: Block allocated in VecCreate_MPI_Private() # line # 497 in /sandbox/bsmith/petsc/src/vec/vec/i\ mpls/mpi/pbvec.c # [0]PETSC ERROR: --------------------- Error Message ------------------------------------------------------\ -------- # [0]PETSC ERROR: Memory corruption: # http://www.mcs.anl.gov/petsc/documentation/installation.html#valgrind # [0]PETSC ERROR: Corrupted memory # [0]PETSC ERROR: See # http://www.mcs.anl.gov/petsc/documentation/faq.html for # trouble # shooting. # [0]PETSC ERROR: Petsc Development GIT revision: # v3.7.5-3057-g2d11e0f GIT Date: 2017-02-07 15:39:04 -0600 # [0]PETSC ERROR: ../ex18 on a arch-single named es by bsmith # Tue # Feb 7 15:46:25 2017 # [0]PETSC ERROR: Configure options --download-ctetgen # --download-exodusii --download-hdf5 --download-mpich \ --download-netcdf PETSC_ARCH=arch-single --with-precision=single # [0]PETSC ERROR: #1 PetscTrFreeDefault() line 287 in # /sandbox/bsmith/petsc/src/sys/memory/mtr.c # [0]PETSC ERROR: #2 VecDestroy_MPI() line 18 in # /sandbox/bsmith/petsc/src/vec/vec/impls/mpi/pdvec.c # [0]PETSC ERROR: #3 VecDestroy() line 406 in # /sandbox/bsmith/petsc/src/vec/vec/interface/vector.c # [0]PETSC ERROR: #4 DMDestroy() line 681 in # /sandbox/bsmith/petsc/src/dm/interface/dm.c # [0]PETSC ERROR: #5 main() line 473 in # /sandbox/bsmith/petsc/src/dm/impls/plex/examples/tests/ex18.c # [0]PETSC ERROR: PETSc Option Table entries: # [0]PETSC ERROR: -dm_view ascii::ascii_info_detail # [0]PETSC ERROR: -interpolate
<val.patch>