Separate test routines from main program

[kilian]

Detaching the unit-test subroutines was identified as a possible way to reduce dependencies of the main program (actually, the top-level 'whizard' module). The unit tests can be called from the whizard executable via command-line options, but the actual routines are re-implemented as stand-alone subroutines in a separate test library. Since the test-routine interface is uniform and trivial, there is no issue with interface checking.

This should discourage meaningless module-file reading and optimization attempts by the compiler, hopefully speeding up the compilation (gfortran).

[kilian]

First step r7013: moved all test calls up into main.f90, so libwhizard.a no longer contains those tests. For further reduction of compile time (for main.f90), I start to move the unit tests as external procedures to the new library libwhizard_test.a.

[kilian]

Not sure how to proceed. I've now exported all unit-tests within whizard-core, not yet committed to trunk.

I think that it makes sense to export the unit tests into a separate library which is accessed only by the main program. However, this doesn't look like speeding up compilation with gfortran. The compilation time for whizard.f90 is reduced, but the compilation time for the test modules have to be added, and this also takes time if they USE many modules themselves. I'm suspicious because the compilation time for main.f90 after the changes is 1:15min on my laptop with or without optimization, unchanged.

For me, it looks like the gfortran compiler is very inefficient in reading and digesting its own .mod files. The dependencies must appear somewhere, so overall compile time can't really be reduced. The only change that should help is to compile with unit tests disabled, then there are strictly less dependencies than previously. In the new scheme, such an option is feasible.

Note that NAG compiles very fast, at least without optimization.

Opinions?

[Correction: the -O0 option does reduce compilation time, but not more than 20%]

[kilian]

OK, another test shows that I can reduce the compilation time for simulations_ut by a factor 3 if the modules are USED only once instead of within every test routine separately. Fine, don't use independent routines but collect them in their own implementation module.

(Why not doing this in the first place: the current plan was to have write them as independent procedures but collect them into a submodule as soon as the feature is available, with reversed dependency direction. An ordinary module is a reasonable workaround.)

In any case, this is a serious drawback of the gfortran compiler.

[Juergen Reuter]

So what are the consequences now of your findings (for the gfortran compiler)? [there is no chance to move away from it]

[kilian]

Replying to jr_reuter:

So what are the consequences now of your findings (for the gfortran compiler)?

That I see no way of significantly reducing gfortran's compilation time for WHIZARD in total. Unless gfortran6 has a new module file format/reader.

I do ask whether a configure option to turn unit tests compilation off would be useful?

[there is no chance to move away from it]

Well, for development I can only recommend NAG. For production the compile time doesn't matter.

[Juergen Reuter]

To put the observations also down here: the linking of libwhizard_ut takes ages, really ages, also as well as the compilation of main.f90. This makes me believe that we maybe probably go better back to the status before the changes made in this ticket.

[Bijan Chokoufe Nejad]

I think a good compromise would be to move the compilation of the unit tests library into the tests DIR (where it might belong anyway). When the program in TESTS has it via _LDADD, it would only be build when make check is issued and one can happily printf debug with the Whizard main also without NAG.

[kilian]

Now I got some data. I compare r7012 (before the modifications to unit tests) with my current local status (exported all tests in process_integration, transforms, whizard-core, sindarin).

I run

make && make -C tests/process_integration clean

to clean the top-level modules and then

time make

to measure the rebuilding of the directories from process_integration upwards, including linking the main program. (The sindarin modules are not rebuilt.) In the new version, module dependencies have been disentangled somewhat, but there are some more independent modules.

The results (compiler options -g -O2 in all cases):

gfortran 4.8.1, r7012:   11m35s
gfortran 4.8.1, current:  8m58s

nagfor 6.0, r7012:        3m16s
nagfor 6.0, current:      3m24s

In other words, it doesn't get worse, and for gfortran there is actually an improvement. I guess that gfortran suffers heavily from module dependencies, while the NAG compile time is dominated by actual compilation and optimization.

I would take this as an encouragement to continue the logical separation of unit tests, but it's not a revolution.

---

The guess for gfortran is supported by the following observation: compile the following module with gfortran 4.8.2, default options:

module modtest1
  private
  type, public :: abcde
   contains
     procedure :: foo
  end type abcde
contains
  subroutine foo (a)
    class(abcde), intent(inout) :: a
  end subroutine foo
end module modtest1

module modtest2
!  use modtest1, only:
  private
end module modtest2

The sizes of the generated .mod files are as expected:

-rw-r--r-- 1 kilian tp 3491 Jun 17 09:27 modtest1.mod
-rw-r--r-- 1 kilian tp  254 Jun 17 09:27 modtest2.mod

Now activate the USE directive in modtest2. The directive does not import anything into modtest2, and modtest2 does not export anything, as before. But now:

-rw-r--r-- 1 kilian tp 3491 Jun 17 09:27 modtest1.mod
-rw-r--r-- 1 kilian tp 3077 Jun 17 09:27 modtest2.mod

Looking at the second .mod file, it repeats almost all of the modtest1 contents even though they are completely inaccessible! This will also happen recursively.

Concluding: no matter how we reorganize the module hierarchy, gfortran will always schlep around heavy .mod files with irrelevant contents. Can anybody check if this is still the case with gfortran 6, or if there is a bugzilla report? In our case this is close to a showstopper.

[kilian]

Replying to bchokoufe:

I think a good compromise would be to move the compilation of the unit tests library into the tests DIR (where it might belong anyway). When the program in TESTS has it via _LDADD, it would only be build when make check is issued and one can happily printf debug with the Whizard main also without NAG.

Yes, that is a possible solution. Currently, the program would require a dummy test library to link properly, have to think about it.

[Bijan Chokoufe Nejad]

I would say going from 11min to 8min is already an acchievement :).

I like the "schlep around" expression a lot ;). We indeed have a lot of ~6 MB mods lying around. No matter how you write your reader, this will take processing time.

I guess it is not a bug but a feature from gfortran's point of view. I mean this is the reason why I always have to add directories to include for nagfor since nagfor doesn't recursively import the other .mods. The possibility to disable this behavior with a switch in gfortran like --no-module-imports would be great though.

It's quite sad that the only statement doesn't change the import behavior at all.

[Bijan Chokoufe Nejad]

Replying to kilian:

Replying to bchokoufe:

I think a good compromise would be to move the compilation of the unit tests library into the tests DIR (where it might belong anyway). When the program in TESTS has it via _LDADD, it would only be build when make check is issued and one can happily printf debug with the Whizard main also without NAG.

Yes, that is a possible solution. Currently, the program would require a dummy test library to link properly, have to think about it.

I would not make a dummy library (I am also against configure time disabling the unit tests. This is quite unflexible. By the time I know I need it, I have to make a new build). Instead just make a new binary like whizard_ut in tests/ that can only execute unit tests. There is no big advantage of having this in the main whizard.

[Bijan Chokoufe Nejad]

Btw dispatch.mod is our alltime heavy weight champion with 19MB.

[kilian]

Replying to bchokoufe:

Btw dispatch.mod is our alltime heavy weight champion with 19MB.

Yes, and this can easily be split into several independent modules, actually. But given the above findings, I doubt that this would improve anything.

The gfortran approach would be fine (always combining all modules in one .mod file) if it would only include accessible names and objects. They probably don't care, but in our case it's the main bottleneck.

I'll go for a separate executable, then. Note that it will take some time before all unit tests find their place in _ut libraries, so whizard_ut will temporarily have to call unit tests also from the core library. But that shouldn't be a problem.

NB: But the potential issue with the MAC linker has to be sorted out ...

[Juergen Reuter]

Good morning, everybody. Going through the discussions, I can comment: yes, I can confirm that I don't want the unit tests depending on a configure option, and yes, I'm in favor of moving executables for the tests to the tests DIR, which will only be compiled when doing make check. gcc does it that way, too, and also some HEP programs. So the decisions made here have my blessings.

[Juergen Reuter]

For the extended tests, it might be interesting to see what gcc is doing specifically for their test suite: ​https://gcc.gnu.org/install/test.html

[kilian]

Replying to jr_reuter:

For the extended tests, it might be interesting to see what gcc is doing specifically for their test suite: ​https://gcc.gnu.org/install/test.html

I guess that they redirect the check target in some parent dir, so that check is not executed, but in its place check-XXX resolves to make check in a subdirectory. By default, automake generates a rule

check: check-recursive

where recursive targets are delegated to all subdirectories.

Anybody has the gcc sources installed?

[Juergen Reuter]

As it looks like, gcc is not using autoconf, automake for the generation of their makefiles. They are using autogen to generate the main Makefiles from Makefile.def and Makefile.tpl in the main directory of gcc. The definition looks like this:

languages = { language=fortran; gcc-check-target=check-fortran;
                                lib-check-target=check-target-libquadmath;
                                lib-check-target=check-target-libgfortran; };

and this is the definition of the check target:

# Check target.

.PHONY: check do-check
check: do-check

# Only include modules actually being configured and built.
.PHONY: check-host
check-host: [+
  FOR host_modules +] \
    maybe-check-[+module+][+
  ENDFOR host_modules +]

.PHONY: check-target
check-target: [+
  FOR target_modules +] \
    maybe-check-target-[+module+][+
  ENDFOR target_modules +]

do-check:
        @: $(MAKE); $(unstage)
        @r=`${PWD_COMMAND}`; export r; \
        s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
        $(MAKE) $(RECURSE_FLAGS_TO_PASS) check-host check-target

and then in gcc/fortran (the analogue of src/fortran) there is a Make-lang.in which contains

check-f95 : check-gfortran
check-fortran : check-gfortran
check-f95-subtargets : check-gfortran-subtargets
check-fortran-subtargets : check-gfortran-subtargets
lang_checks += check-gfortran
lang_checks_parallelized += check-gfortran
# For description see comment above check_gcc_parallelize in gcc/Makefile.in.
check_gfortran_parallelize = 10000

I don't know whether this is of any help.

[kilian]

If one can overwrite a default target (check) in automake, we can tweak this in a similar way. Or just leave it as-is (check = check all), but invent all new check targets for everyday use.

[kilian]

If I understand the docs correctly, it's not possible to override a default recursive target, but we can define our own recursive targets.h Would that be a solution? Something like check-min, or check-ut, or ... suggestions?

[Juergen Reuter]

Unit tests and functional tests should always be tested, so in principle we don't need a separate check-target for those. On the other hand, it doesn't hurt to have one. For the extended tests, we probably want to have several like e.g. check-SUSY, check-ILC, check-FORMATS etc.

[Bijan Chokoufe Nejad]

With the new directory structure, we basically already have the possibility to only do functional or unit tests by just entering the corresponding subdir. So I think it boils down to new targets for the extended tests after all.

[Bijan Chokoufe Nejad]

We could also remove extended_tests from the SUBDIRS and then set up new targets like this:

check-extended:
    make -C extended_tests check

check-extended-SUSY:
    make -C extended_tests/SUSY check

This would allow us to use the default TESTS but then we also have to make sure that it is still cleaned.

[kilian]

Yes, but excluding from subdirs means that the Makefile will not be controlled by Automake there, which introduces much more trouble.

[kilian]

New recursive targets are handled like this: introduce check-extended according to Automake docs, then in the appropriate subdir:

check-extended-local: check

and all is fine, so far.

My concern is how to reduce the effect of default make check (as issued from topdir).

[Juergen Reuter]

Replying to kilian:

Yes, but excluding from subdirs means that the Makefile will not be controlled by Automake there, which introduces much more trouble.

Lol, thanks, you were faster on that one.

[Bijan Chokoufe Nejad]

Yes it might be even more trouble to exclude it. Just as an FYI, this is how my attempt looks to emulate make check for a non-default target

EXTRA_DIST += extracheck_combiner.sh produce_trs.sh
.run.run.extra.trs:
	$(SHELL) $(srcdir)/produce_trs.sh $< >| $@
clean-extracheck_results:
	rm -f *.extra.trs
.PHONY: clean-extracheck_results

EXTRACHECK_SHOWER_RUN = extra_shower_1.run extra_shower_2.run
EXTRACHECK_SHOWER_SH = $(EXTRACHECK_SHOWER_RUN:.run=.sh)
EXTRACHECK_SHOWER_TRS = $(EXTRACHECK_SHOWER_RUN:.run=.run.extra.trs)
.SECONDARY: $(EXTRACHECK_SHOWER_RUN)
extracheck_shower: $(BUILT_SOURCES) $(EXTRACHECK_SHOWER_TRS)
	$(SHELL) $(srcdir)/extracheck_combiner.sh "extra_shower_*.run.extra.trs"

I haven't checked that everything is also distcleaned but so far it works quite well. The second block has to be repeated for each suite (shower, NLO, SUSY, etc.). I added .SECONDARY to the runs to be able to rerun them. make clean-extracheck_results is for convenience.

Considering the recurisve target. Does it really make sense that make should go in every directory to look for check-extended although we know where it is?

[kilian]

Try to put 'extended' in DIST_SUBDIRS instead of SUBDIRS. This should save part of the trouble.

[Juergen Reuter]

Well, we have to agree on a clean way to do this, for the old extended check targets and the new one. The sooner, the better.

[kilian]

OK, so those are the recursive targets for DIST_SUBDIRS:

dist distclean maintainer-clean

and those are the other recursive targets:

all 
check
cscopelist ctags dvi html info
install-data install-dvi install-exec 
install-html install-info install-pdf install-ps
install installcheck installdirs 
pdf ps tags 
uninstall

and also

mostlyclean clean

So if we reduce the extra test dirs to DIST_SUBDIRS, we must explicitly write rules for the targets that are no longer recursive. As I see it, this will be only clean, or does anything get installed?

Note that check inside the subdir should work as normal, it just won't be called by recursive check.

Regarding extra recursive targets: yes, might be redundant, but it would at least be possible to issue them from any parent of the actual test directory, if they are recursive.

[Bijan Chokoufe Nejad]

Alright, sounds good. Nothing should be installed, so we should be good to go with DIST_SUBDIRS. The only question that remains is whether we put the different suites in different directories within extended_tests then or if we select them via TESTS=.

[kilian]

With different directories, you can just redirect

check-ILC-local: check

in the specific subdir and all is well. So, there should be meaningful groups of test suites.

(Probably the make all target has also to be supported, if BUILT_SOURCES is not sufficient.)

[Juergen Reuter]

The only open question(s) now is/are: 1) To we have ilc_tests, susy_tests, format_tests etc. as subdirs of tests (like unit_tests and functional_tests) or do we put them one layer lower in a separate directory extended_tests. My gut feeling is more for the first solution. 2) who does it, WK or BACN?

[kilian]

Replying to jr_reuter:

The only open question(s) now is/are: 1) To we have ilc_tests, susy_tests, format_tests etc. as subdirs of tests (like unit_tests and functional_tests) or do we put them one layer lower in a separate directory extended_tests. My gut feeling is more for the first solution.

Yes, only one level of subdirs.

2) who does it, WK or BACN?

Right now, I'd like to handle the unit tests first. If BACN wants to do the extended tests, no objection.

[kilian]

With r7062, all unit tests are exported in a separate library which will only be built when make check is called. This should settle the main issue.

There are some minor issues remaining:

1. Enable the extended tests via a new check target - right now the directory is configured but not built (BACN?)
2. Add more extended tests as appropriate
3. Copy only .ref files where they are needed (optional enhancement)

I'd like to close this ticket, how to proceed?

[Bijan Chokoufe Nejad]

Replying to kilian:

With r7062, all unit tests are exported in a separate library which will only be built when make check is called. This should settle the main issue.

There are some minor issues remaining:

1. Enable the extended tests via a new check target - right now the directory is configured but not built (BACN?)
2. Add more extended tests as appropriate
3. Copy only .ref files where they are needed (optional enhancement)

I'd like to close this ticket, how to proceed?

I am still busy with 2.) in order to make Mikaels Patch work for all cases. So if you could do 1.), I will adapt it for the further test suites.

[kilian]

@BACN: yes, I'll try. So we don't interfere right now.

[kilian]

r7065: The recursive Make target 'check-extended' is now enabled.

In principle, it can be used anywhere in the tree by setting up a 'check-extended-local' rule. Currently, this is the case only in 'tests', where it redirects to 'make check' in the 'tests/extended_tests' subdir. The latter is not touched by 'make check' otherwise.

Since this executes a sub-make in the target directory, it also allows us to make the execution dependent on environment variables. See the followup ticket #736.

I leave the unconditional copying of .ref files as WONTFIX, since this doesn't bother us too much, currently.