Skip to content

Commit

Permalink
Fix test for invalid compilation tool on base class
Browse files Browse the repository at this point in the history
Try changing order that msvc compiler initialized before conda
Remove CFLAGS from compilation arguments in example makefiles to prevent incorrect linking on mac arm64 to x86-64
Return results from find_all in sorted order
Add -fPIC flag to fortran interface compilation arguments for GNU
Allow for basetool to be None in case of missing compilation tools
Handle case of clang on windows when parsing version
  • Loading branch information
langmm committed May 29, 2024
1 parent 0a51fd1 commit 4436807
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 40 deletions.
40 changes: 20 additions & 20 deletions .github/workflows/test-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ jobs:
'
shell: bash -l {0}
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
Expand All @@ -457,10 +461,6 @@ jobs:
miniforge-variant: Mambaforge
python-version: ${{ matrix.python-version }}
use-mamba: true
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- continue-on-error: true
id: conda_build
if: matrix.install-method == 'conda' || matrix.install-method == 'mamba'
Expand Down Expand Up @@ -845,6 +845,10 @@ jobs:
'
shell: bash -l {0}
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
Expand All @@ -855,10 +859,6 @@ jobs:
miniforge-variant: Mambaforge
python-version: ${{ matrix.python-version }}
use-mamba: true
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- continue-on-error: true
id: conda_build
if: matrix.install-method == 'conda' || matrix.install-method == 'mamba'
Expand Down Expand Up @@ -1219,6 +1219,10 @@ jobs:
'
shell: bash -l {0}
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
Expand All @@ -1229,10 +1233,6 @@ jobs:
miniforge-variant: Mambaforge
python-version: ${{ matrix.python-version }}
use-mamba: true
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- continue-on-error: true
id: conda_build
if: matrix.install-method == 'conda' || matrix.install-method == 'mamba'
Expand Down Expand Up @@ -1574,6 +1574,10 @@ jobs:
'
shell: bash -l {0}
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
Expand All @@ -1584,10 +1588,6 @@ jobs:
miniforge-variant: Mambaforge
python-version: ${{ matrix.python-version }}
use-mamba: true
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- continue-on-error: true
id: conda_build
if: matrix.install-method == 'conda' || matrix.install-method == 'mamba'
Expand Down Expand Up @@ -1928,6 +1928,10 @@ jobs:
'
shell: bash -l {0}
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
Expand All @@ -1938,10 +1942,6 @@ jobs:
miniforge-variant: Mambaforge
python-version: ${{ matrix.python-version }}
use-mamba: true
- if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') &&
runner.os == 'Windows' && env.INSTALLC == 1
name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
- continue-on-error: true
id: conda_build
if: matrix.install-method == 'conda' || matrix.install-method == 'mamba'
Expand Down
5 changes: 3 additions & 2 deletions tests/drivers/test_ModelDriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from yggdrasil import platform, constants
from yggdrasil.drivers.ModelDriver import ModelDriver
from yggdrasil.drivers.CompiledModelDriver import (
CompiledModelDriver, InvalidCompilationTool)
CompiledModelDriver, InvalidCompilationTool, get_tool_registry)
from yggdrasil.drivers.InterpretedModelDriver import InterpretedModelDriver


Expand All @@ -20,8 +20,9 @@ def test_ModelDriver_implementation():
ModelDriver.executable_command(None)
with pytest.raises(NotImplementedError):
ModelDriver.is_library_installed(None)
get_tool_registry()._init_languages(['c++'])
with pytest.raises(InvalidCompilationTool):
print(CompiledModelDriver.get_tool('compiler'))
CompiledModelDriver.get_tool('compiler')
with pytest.raises(NotImplementedError):
CompiledModelDriver.language_executable()
with pytest.raises(NotImplementedError):
Expand Down
14 changes: 9 additions & 5 deletions utils/test-install-base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ jobs:
run: |
echo "PATH=$(python utils/setup_test_env.py prune-path)" >> $GITHUB_ENV
shell: bash -l {0}
# Try initializing compiler first
- name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') && runner.os == 'Windows' && env.INSTALLC == 1
- name: (CONDA) Set up miniconda base environment
uses: conda-incubator/setup-miniconda@v3
if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba')
Expand All @@ -247,11 +251,11 @@ jobs:
# initialization
# This step might need to stay here so that dev prompt comes before
# conda?
- name: (WINDOWS,CONDA) Set up MSVC Compiler
uses: ilammy/msvc-dev-cmd@v1
if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') && runner.os == 'Windows' && env.INSTALLC == 1
# with:
# toolset: 14.0
# - name: (WINDOWS,CONDA) Set up MSVC Compiler
# uses: ilammy/msvc-dev-cmd@v1
# if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') && runner.os == 'Windows' && env.INSTALLC == 1
# # with:
# # toolset: 14.0
# - name: (UNIX,CONDA) Add miniconda bin to path
# if: (matrix.install-method == 'conda' || matrix.install-method == 'mamba') && runner.os != 'Windows'
# run: |
Expand Down
6 changes: 4 additions & 2 deletions yggdrasil/drivers/CMakeModelDriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,10 @@ def create_imports(cls, dep, imp, products=None, overwrite=False,
logger.debug(f"CREATE_IMPORTS {dep} {imp}")
buildfile = dep['buildfile']
if not os.path.isabs(buildfile):
buildfile = dep._relative_to_directory(buildfile)
logger.info(f"CREATE_IMPORTS {dep}: {buildfile}")
buildfile = dep._relative_to_directory(
buildfile, directory=dep.get('working_dir', None))
logger.info(f"CREATE_IMPORTS {dep}: {buildfile}\n"
f"source_dir: {dep.get('source_dir', None)}")
target = dep.get('target', '${PROJECT_NAME}')
if products is None:
products = tools.IntegrationPathSet(overwrite=overwrite)
Expand Down
4 changes: 2 additions & 2 deletions yggdrasil/drivers/CModelDriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,8 @@ def get_flags(cls, *args, **kwargs):
# https://bugs.llvm.org/show_bug.cgi?id=44813
# https://reviews.llvm.org/D71579
# https://reviews.llvm.org/D74784
ver = cls.tool_version().split()[-1]
if int(ver.split('.')[0]) >= 10:
ver = cls.tool_version().split()[-1].split('.')[0]
if ver.isnumeric() and int(ver) >= 10:
ld_version = LDLinker.tool_version()
if float(ld_version.split('.')[0]) < 520: # pragma: version
# No longer covered as the default conda
Expand Down
8 changes: 7 additions & 1 deletion yggdrasil/drivers/CompiledModelDriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ def _check_driver(self, language=None, driver=None,
driver = None
elif driver:
language = driver.language
if not language:
raise InvalidCompilationTool(f"Driver {driver} does not "
f"does not have a language "
f"set")
if language and (not skip_driver) and (not driver):
driver = import_component('model', language)
return (language, driver)
Expand Down Expand Up @@ -1873,9 +1877,11 @@ def all_parameters(self, generalize=False, only_active=False):
tooltypes = _tool_types
elif only_active:
tooltypes = self.active_tools()
else:
elif self.basetool:
tooltypes = [self.basetool.tooltype]
tooltypes += self.basetool.associated_tooltypes
else:
tooltypes = []
for k in tooltypes:
out += self.tool_parameters(k, generalize=generalize)
if generalize or only_active:
Expand Down
3 changes: 3 additions & 0 deletions yggdrasil/drivers/FortranModelDriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ class FortranModelDriver(CompiledModelDriver):
if tools.get_conda_prefix() else False
}
}
},
'Linux': {
'compiler_flags': ['-fPIC'],
}
}},
'c_wrappers': {
Expand Down
4 changes: 2 additions & 2 deletions yggdrasil/examples/backwards/src/Makefile_linux
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ $(ODIR)/%.o: %.c
$(CC) -c $(CFLAGS) $< -o $@

backwards_modelA: $(ODIR)/backwards_modelA.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

backwards_modelB: $(ODIR)/backwards_modelB.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

.PHONY: clean
clean:
Expand Down
4 changes: 2 additions & 2 deletions yggdrasil/examples/gs_lesson4/src/Makefile_linux
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ $(ODIR)/%.o: %.c
$(CC) -c $(CFLAGS) $< -o $@

gs_lesson4_modelA: $(ODIR)/gs_lesson4_modelA.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

gs_lesson4_modelB: $(ODIR)/gs_lesson4_modelB.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

.PHONY: clean
clean:
Expand Down
4 changes: 2 additions & 2 deletions yggdrasil/examples/model_error_with_io/src/Makefile_linux
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ $(ODIR)/%.o: %.c
$(CC) -c $(CFLAGS) $< -o $@

model_error_with_io_modelA: $(ODIR)/model_error_with_io_modelA.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

model_error_with_io_modelB: $(ODIR)/model_error_with_io_modelB.o
$(CC) -o $@ $^ $(CFLAGS) $(LIBS)
$(CC) -o $@ $^ $(LIBS)

.PHONY: clean
clean:
Expand Down
5 changes: 3 additions & 2 deletions yggdrasil/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,9 @@ def find_all(name, path, verification_func=None, use_regex=False,
if use_regex:
result = [
x for x in result if re.fullmatch(regex_pattern, x)]
result = [os.path.normcase(os.path.normpath(bytes2str(m)))
for m in result]
result = sorted(
[os.path.normcase(os.path.normpath(bytes2str(m)))
for m in result])
if verification_func is not None:
result = [x for x in result if verification_func(x)]
return result
Expand Down

0 comments on commit 4436807

Please sign in to comment.