PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
202 stars 235 forks source link

build: Reduce Workflow Time for CI GitHub Action #3176

Closed allgandalf closed 1 year ago

allgandalf commented 1 year ago

Description

In this PR, I have updated the Makefile and created phony targets for each base, module and model, also I have updated the CI.yml such that these checks will run in parallel, there by reducing the time from nearly 37 minutes - 1 hour to roughly around 15 to 16 minutes, the main checks for base,module and model would take around 13 - 14 minutes and the extra 1 - 2 minutes would be required to download the utilities (Which totally depends on the environment).

Motivation and Context

Currently GitHub Actions will check to see if there are newer versions of the packages installed. Biggest problem was that right now we run out of minutes and compiles take a lot longer than needed.

Review Time Estimate

Types of changes

Checklist:

cc: @robkooper
infotroph commented 1 year ago

If I'm understanding the approach right, I think you could achieve the same effect by defining new phony targets in the existing Makefile, one for each operation on a subset of packages:

--- a/Makefile
+++ b/Makefile
@@ -96,10 +96,20 @@ depends = .doc/$(1) .install/$(1) .check/$(1) .test/$(1)

 ### Rules

-.PHONY: all install check test document shiny
+.PHONY: all install check test document shiny \
+               install_base install_models install_modules \
[snip snip snip]
+               document_base document_models document_modules

 all: install document

+install_base: $(BASE_I)
+install_models: $(MODELS_I)
[snip snip snip]
+document_models: $(MODELS_D)
+document_modules: $(MODULES_D)
+
 document: $(ALL_PKGS_D) .doc/base/all
 install: $(ALL_PKGS_I) .install/base/all
 check: $(ALL_PKGS_C) .check/base/all

Then each operation can call the command it needs, e.g. make install_base or make document_modules.

infotroph commented 1 year ago

Followup thought: there's a good chance we don't need all 12 of those like my previous comment implies. Maybe for CI purposes it's sufficient to define check_* and leave install/test/doc as single commands that work on all packages.

allgandalf commented 1 year ago

thank you @mdietze , @infotroph for reviewing this PR, using phony targets is surely a workaround, I have made the necessary changes and also removed the individual makefiles. Do check the latest changes once :)