Skip to content

Cleanup the use of find in makefile - #1567

Merged
rok-cesnovar merged 4 commits into
developfrom
fix_windows_makefile
Jan 31, 2020
Merged

Cleanup the use of find in makefile#1567
rok-cesnovar merged 4 commits into
developfrom
fix_windows_makefile

Conversation

@rok-cesnovar

@rok-cesnovar rok-cesnovar commented Dec 30, 2019

Copy link
Copy Markdown
Member

Summary

This PR fixes the Stan Math side of stan-dev/cmdstan#786

The issue is that our makefiles rely on the gnu find command that is not available on Windows.

This PR introduces a makefile subroutine that calls dir on Windows and find on Unix systems.

ifeq ($(OS),Windows_NT)
findfiles=$(shell cmd.exe /C "dir /b /s $(subst /,,$1)$2 2>NUL")
else
findfiles=$(shell find $1 -type f -name $2)
endif

The 2>NUL is there because if you search for files that dont exist, dir will output a File not found error. So if you called make clean twice in a row it would output a bunch of File not founds.

This PR introduces a findfiles function that is a recursive version of the makefiles wildcard function.
findfiles=$(foreach d,$(wildcard $(addsuffix *,$(1))),$(call findfiles,$(d)/,$(2)) $(filter $(subst *,%,$(2)),$(d)))

This PR also excludes the generaton of distribution tests on Windows. This means that make generate-tests will not be available on Windows by default. This functionality is currently not working on Windows and just produces a bunch of warnings. Making this functionality available on Windows should be addressed separately. That requires a larger overhaul and since its a functionality aimed at developers its not something that needs to be addressed before the next release. The unnecessary warnings seen by users compiling models is a priority in my opinion.

Tests

/

Side Effects

/

@stan-buildbot

Copy link
Copy Markdown
Contributor

(stat_comp_benchmarks/benchmarks/gp_pois_regr/gp_pois_regr.stan, 1.0)
(stat_comp_benchmarks/benchmarks/low_dim_corr_gauss/low_dim_corr_gauss.stan, 0.98)
(stat_comp_benchmarks/benchmarks/irt_2pl/irt_2pl.stan, 0.99)
(stat_comp_benchmarks/benchmarks/pkpd/one_comp_mm_elim_abs.stan, 0.97)
(stat_comp_benchmarks/benchmarks/eight_schools/eight_schools.stan, 0.99)
(stat_comp_benchmarks/benchmarks/gp_regr/gp_regr.stan, 1.02)
(stat_comp_benchmarks/benchmarks/arK/arK.stan, 0.91)
(performance.compilation, 1.01)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan, 0.99)
(stat_comp_benchmarks/benchmarks/low_dim_gauss_mix/low_dim_gauss_mix.stan, 0.99)
(stat_comp_benchmarks/benchmarks/sir/sir.stan, 1.07)
(stat_comp_benchmarks/benchmarks/pkpd/sim_one_comp_mm_elim_abs.stan, 1.0)
(stat_comp_benchmarks/benchmarks/garch/garch.stan, 1.01)
(stat_comp_benchmarks/benchmarks/gp_regr/gen_gp_data.stan, 0.99)
(stat_comp_benchmarks/benchmarks/arma/arma.stan, 1.01)
Result: 0.99512462713
Commit hash: fb7224b

@rok-cesnovar
rok-cesnovar requested a review from syclik January 5, 2020 09:43

@syclik syclik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's slick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants