Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update build scripts to always run tests for all MXNet engine types #71

Open
wants to merge 6 commits into
base: v0.x.x
Choose a base branch
from
15 changes: 14 additions & 1 deletion Build.mak
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,25 @@ endif
download-mnist: $C/script/download-mnist
$(call exec,sh $(if $V,,-x) $^,$(MNIST_DATA_DIR),$^)

# helper function to run tests
run_test = $(call exec,MXNET_ENGINE_TYPE=$2 $1,$1,$2)

# extra build dependencies for integration tests
$O/test-mxnet: override LDFLAGS += -lz
$O/test-mxnet: override DFLAGS += -debug=MXNetHandleManualFree

# extra runtime dependencies for integration tests
$O/test-mxnet.stamp: override ITFLAGS += $(MNIST_DATA_DIR)
$O/test-mxnet.stamp: download-mnist
$O/test-mxnet.stamp: $O/test-mxnet download-mnist
$(call run_test,$<,NaiveEngine)
$(call run_test,$<,ThreadedEngine)
$(call run_test,$<,ThreadedEnginePerDevice)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It occurs to me that an even better option would be to have a variable defining the supported engines (set in Config.mak if not already defined) and just foreach over the entries in that variable. But I'm finding it tricky to work out how best to do that :-\

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically: how to implement an appropriate for loop. Although I guess a simple, manual way is to have a separate run_test_with_all_engines function which takes a single parameter (the test to run).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried with:

run_test_with_all_engines = $(foreach engine,$(SUPPORTED_MXNET_ENGINES),$(call run_test,$1,$(engine)))

... but for some reason, this only ever picks up on the first entry in the SUPPORTED_MXNET_ENGINES variable.

Choose a reason for hiding this comment

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

BTW, the dependency of $O/test-mxnet is added automatically by Makd, you don't need to put it explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I assumed, but it didn't work without it. :-\

$Vtouch $@

$O/%unittests: override LDFLAGS += -lz

$O/allunittests.stamp: $O/allunittests
$(call run_test,$<,NaiveEngine)
$(call run_test,$<,ThreadedEngine)
$(call run_test,$<,ThreadedEnginePerDevice)
$Vtouch $@