#730 closed task (fixed)
Separate test routines from main program
Reported by: | kilian | Owned by: | kilian |
---|---|---|---|
Priority: | P1 | Milestone: | v2.2.7 |
Component: | core | Version: | 2.2.6 |
Severity: | normal | Keywords: | |
Cc: |
Description
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).
Change History (38)
comment:1 Changed 10 years ago by
Status: | new → assigned |
---|
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
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%]
comment:4 Changed 10 years ago by
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.
comment:5 follow-up: 6 Changed 10 years ago by
So what are the consequences now of your findings (for the gfortran compiler)? [there is no chance to move away from it]
comment:6 Changed 10 years ago by
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.
comment:7 Changed 10 years ago by
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.
comment:8 follow-up: 10 Changed 10 years ago by
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.
comment:9 Changed 10 years ago by
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.
comment:10 follow-up: 12 Changed 10 years ago by
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 whenmake 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.
comment:11 Changed 10 years ago by
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.
comment:12 Changed 10 years ago by
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 whenmake 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
.
comment:13 follow-up: 14 Changed 10 years ago by
Btw dispatch.mod
is our alltime heavy weight champion with 19MB.
comment:14 Changed 10 years ago by
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 ...
comment:15 Changed 10 years ago by
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.
comment:16 follow-up: 17 Changed 10 years ago by
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
comment:17 Changed 10 years ago by
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?
comment:18 Changed 10 years ago by
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.
comment:19 Changed 10 years ago by
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.
comment:20 follow-up: 23 Changed 10 years ago by
If I understand the docs correctly, it's not possible to override a default recursive target, but we can define our own recursive targets. Would that be a solution? Something like check-min, or check-ut, or ... suggestions?
comment:21 Changed 10 years ago by
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.
comment:22 Changed 10 years ago by
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.
comment:23 Changed 10 years ago by
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 clean
ed.
comment:24 follow-up: 26 Changed 10 years ago by
Yes, but excluding from subdirs means that the Makefile will not be controlled by Automake there, which introduces much more trouble.
comment:25 Changed 10 years ago by
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).
comment:26 Changed 10 years ago by
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.
comment:27 Changed 10 years ago by
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 distclean
ed 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?
comment:28 Changed 10 years ago by
Try to put 'extended' in DIST_SUBDIRS instead of SUBDIRS. This should save part of the trouble.
comment:29 Changed 10 years ago by
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.
comment:30 Changed 10 years ago by
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.
comment:31 Changed 10 years ago by
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=
.
comment:32 Changed 10 years ago by
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.)
comment:33 follow-up: 34 Changed 10 years ago by
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?
comment:34 Changed 10 years ago by
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.
comment:35 follow-up: 36 Changed 10 years ago by
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:
- Enable the extended tests via a new check target - right now the directory is configured but not built (BACN?)
- Add more extended tests as appropriate
- Copy only .ref files where they are needed (optional enhancement)
I'd like to close this ticket, how to proceed?
comment:36 Changed 10 years ago by
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:
- Enable the extended tests via a new check target - right now the directory is configured but not built (BACN?)
- Add more extended tests as appropriate
- 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.
comment:38 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.
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.