Increase functionality of try_toolchain #2539

ocaisa · 2018-07-25T12:28:44Z

The goal here is to increase the scope of --try-toolchain-* so that it can handle mapping the entire hierarchy of toolchains from the initial to the target toolchain.

Add toolchain capabilities to the result of get_toolchain_hierarchy
Create a mapping from source hierarchy to target hierarchy
Make sure that the entire dep tree is mapped to the new hierarchy
Ensure that dep toolchains are also updated if explicitly named
Need to handle case where there needs to be a binutils update in an easyconfig that depends on GCCcore

…hains hierarchy

houndci-bot · 2018-07-25T12:28:56Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['blas_family'] = None
+                try:
+                    toolchain['lapack_family'] = tc.lapack_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-25T12:28:56Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['mpi_family'] = None
+                try:
+                    toolchain['blas_family'] = tc.blas_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-25T12:28:56Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['compiler_family'] = None
+                try:
+                    toolchain['mpi_family'] = tc.mpi_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-25T12:28:56Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['blas_family'] = None
+                try:
+                    toolchain['lapack_family'] = tc.lapack_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-25T12:28:56Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['mpi_family'] = None
+                try:
+                    toolchain['blas_family'] = tc.blas_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-25T12:28:57Z

easybuild/framework/easyconfig/easyconfig.py

+                    toolchain['compiler_family'] = None
+                try:
+                    toolchain['mpi_family'] = tc.mpi_family()
+                except:


do not use bare except'

houndci-bot · 2018-07-26T08:33:50Z

easybuild/framework/easyconfig/tweak.py

+    for toolchain_spec in initial_tc_hierarchy:
+        tc_mapping[toolchain_spec['name']] = match_minimum_tc_specs(toolchain_spec, target_tc_hierarchy)
+
+    return tc_mapping


no newline at end of file

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/tweak.py

+                      source_tc_spec['compiler_family'], target_compiler_family)
+
+
+    return minimal_matching_toolchain


too many blank lines (2)

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/tweak.py

+                target_compiler_family = target_tc_spec['compiler_family']
+
+
+    if not minimal_matching_toolchain:


too many blank lines (2)

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/tweak.py

+
+    return can_map
+
+def match_minimum_tc_specs(source_tc_spec, target_tc_hierarchy):


expected 2 blank lines, found 1

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/tweak.py

+    """
+    can_map = True
+    # Check they have same capabilities
+    for key in ['compiler_family', 'mpi_family','blas_family', 'lapack_family', 'cuda']:


missing whitespace after ','

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/tweak.py

@@ -615,3 +616,64 @@ def obtain_ec_for(specs, paths, fp=None):
        return res
    else:
        raise EasyBuildError("No easyconfig found for requested software, and also failed to generate one.")
+
+def compare_toolchain_specs(source_tc_spec, target_tc_spec):


expected 2 blank lines, found 1

houndci-bot · 2018-07-26T08:33:51Z

easybuild/framework/easyconfig/easyconfig.py

+                if 'CUDA_CC' in tc.variables:
+                    toolchain['cuda'] = True
+                else:
+                    toolchain['cuda'] = None # Useful to have it consistent with the rest


at least two spaces before inline comment

houndci-bot · 2018-07-26T08:33:52Z

easybuild/framework/easyconfig/tweak.py

+    for toolchain_spec in initial_tc_hierarchy:
+        tc_mapping[toolchain_spec['name']] = match_minimum_tc_specs(toolchain_spec, target_tc_hierarchy)
+
+    return tc_mapping


no newline at end of file

houndci-bot · 2018-07-26T08:33:52Z

easybuild/framework/easyconfig/tweak.py

+                      source_tc_spec['compiler_family'], target_compiler_family)
+
+
+    return minimal_matching_toolchain


too many blank lines (2)

houndci-bot · 2018-07-26T08:33:52Z

easybuild/framework/easyconfig/tweak.py

+                target_compiler_family = target_tc_spec['compiler_family']
+
+
+    if not minimal_matching_toolchain:


too many blank lines (2)

houndci-bot · 2018-07-26T08:33:52Z

easybuild/framework/easyconfig/tweak.py

+
+    return can_map
+
+def match_minimum_tc_specs(source_tc_spec, target_tc_hierarchy):


expected 2 blank lines, found 1

houndci-bot · 2018-07-26T08:33:53Z

easybuild/framework/easyconfig/tweak.py

+    """
+    can_map = True
+    # Check they have same capabilities
+    for key in ['compiler_family', 'mpi_family','blas_family', 'lapack_family', 'cuda']:


missing whitespace after ','

houndci-bot · 2018-07-26T08:33:53Z

easybuild/framework/easyconfig/tweak.py

@@ -615,3 +616,64 @@ def obtain_ec_for(specs, paths, fp=None):
        return res
    else:
        raise EasyBuildError("No easyconfig found for requested software, and also failed to generate one.")
+
+def compare_toolchain_specs(source_tc_spec, target_tc_spec):


expected 2 blank lines, found 1

houndci-bot · 2018-07-26T08:33:53Z

easybuild/framework/easyconfig/easyconfig.py

+                if 'CUDA_CC' in tc.variables:
+                    toolchain['cuda'] = True
+                else:
+                    toolchain['cuda'] = None # Useful to have it consistent with the rest


at least two spaces before inline comment

ocaisa · 2018-07-26T09:55:07Z

@boegel The last three points will require a major change in how --try-* works, I need to actually parse the easyconfigs so there are no templates and go in and replace values, i.e, we would need to replace the regex.

ocaisa · 2018-07-26T14:10:50Z

easybuild/framework/easyconfig/tweak.py

+    for target_tc_spec in reversed(target_tc_hierarchy):
+        if compare_toolchain_specs(source_tc_spec, target_tc_spec):
+            # GCCcore has compiler capabilities but should only be used if the original toolchain was also GCCcore
+            if (source_tc_spec['name'] != 'GCCcore' and target_tc_spec['name'] != 'GCCcore')\


I don't like the hardcoded 'GCCcore' but I also don't know where I can inherit that from

…re GCCcore does not appear in the target)

houndci-bot · 2018-07-30T11:15:16Z

easybuild/framework/easyconfig/tweak.py

+
+    # TODO Replace the binutils version (if necessary)
+
+    # TODO Determine the name of the modified easyconfig and dump it target_dir


no newline at end of file

houndci-bot · 2018-07-30T11:15:16Z

easybuild/framework/easyconfig/tweak.py

+
+    return resolve_dependencies(ec, modtool)
+
+def map_toolchain_hierarchies(source_toolchain, target_toolchain, modtool):


expected 2 blank lines, found 1

houndci-bot · 2018-07-30T11:15:16Z

easybuild/framework/easyconfig/tweak.py

+
+    return minimal_matching_toolchain
+
+def get_dep_tree_of_toolchain(toolchain_spec, modtool):


expected 2 blank lines, found 1

houndci-bot · 2018-07-30T12:27:56Z

test/framework/tweak.py

+        })
+        get_toolchain_hierarchy.clear()
+        gcc_binutils_tc = {'name': 'GCC', 'version': '4.9.3-2.25'}
+        iccifort_binutils_tc = {'name': 'iccifort', 'version': '2016.1.150-GCC-4.9.3-2.25'}


local variable 'iccifort_binutils_tc' is assigned to but never used

houndci-bot · 2018-07-30T12:27:56Z

test/framework/tweak.py

+            'robot_path': test_easyconfigs,
+        })
+        get_toolchain_hierarchy.clear()
+        gcc_binutils_tc = {'name': 'GCC', 'version': '4.9.3-2.25'}


local variable 'gcc_binutils_tc' is assigned to but never used

houndci-bot · 2018-07-30T12:27:56Z

easybuild/framework/easyconfig/tweak.py

+        targetdir = tempfile.gettempdir()
+    tweaked_spec = os.path.join(target_dir, ec_filename)
+    parsed_ec.dump(tweaked_spec)
+    _log.debug("Dumped easyconfig tweaked via --try-toolchain* to %s", tweaked_spec)


no newline at end of file

houndci-bot · 2018-07-30T12:27:57Z

easybuild/framework/easyconfig/tweak.py

@@ -114,13 +136,23 @@ def tweak(easyconfigs, build_specs, modtool, targetdirs=None):
        # prepended path so that they are found first).
        # easyconfig files for dependencies are also generated but not included, they will be resolved via --robot
        # either from existing easyconfigs or, if that fails, from easyconfigs in the appended path
+
+        modifying_toolchain = False # TODO Remove this to enable all tests


at least two spaces before inline comment

houndci-bot · 2018-07-30T12:27:58Z

test/framework/tweak.py

+        })
+        get_toolchain_hierarchy.clear()
+        gcc_binutils_tc = {'name': 'GCC', 'version': '4.9.3-2.25'}
+        iccifort_binutils_tc = {'name': 'iccifort', 'version': '2016.1.150-GCC-4.9.3-2.25'}


local variable 'iccifort_binutils_tc' is assigned to but never used

houndci-bot · 2018-07-30T12:27:58Z

test/framework/tweak.py

+            'robot_path': test_easyconfigs,
+        })
+        get_toolchain_hierarchy.clear()
+        gcc_binutils_tc = {'name': 'GCC', 'version': '4.9.3-2.25'}


local variable 'gcc_binutils_tc' is assigned to but never used

houndci-bot · 2018-07-30T12:27:58Z

easybuild/framework/easyconfig/tweak.py

+        targetdir = tempfile.gettempdir()
+    tweaked_spec = os.path.join(target_dir, ec_filename)
+    parsed_ec.dump(tweaked_spec)
+    _log.debug("Dumped easyconfig tweaked via --try-toolchain* to %s", tweaked_spec)


no newline at end of file

houndci-bot · 2018-07-30T12:27:58Z

easybuild/framework/easyconfig/tweak.py

@@ -114,13 +136,23 @@ def tweak(easyconfigs, build_specs, modtool, targetdirs=None):
        # prepended path so that they are found first).
        # easyconfig files for dependencies are also generated but not included, they will be resolved via --robot
        # either from existing easyconfigs or, if that fails, from easyconfigs in the appended path
+
+        modifying_toolchain = False # TODO Remove this to enable all tests


at least two spaces before inline comment

houndci-bot · 2018-07-30T18:25:29Z

easybuild/framework/easyconfig/tweak.py

+    parsed_ec['ec'].dump(tweaked_spec)
+    _log.debug("Dumped easyconfig tweaked via --try-toolchain* to %s", tweaked_spec)
+
+    return tweaked_spec


no newline at end of file

houndci-bot · 2018-07-30T18:25:29Z

easybuild/framework/easyconfig/tweak.py

+    parsed_ec['ec'].dump(tweaked_spec)
+    _log.debug("Dumped easyconfig tweaked via --try-toolchain* to %s", tweaked_spec)
+
+    return tweaked_spec


no newline at end of file

easybuild/framework/easyconfig/easyconfig.py

boegel · 2018-09-11T15:39:17Z

easybuild/framework/easyconfig/tweak.py

+    Get the dependency tree of a toolchain
+
+    :param toolchain_spec: toolchain spec to get the dependencies of
+    :param modtool: module tool used


clarify that this should be an instance of the ModulesTool class?

test/framework/tweak.py

easybuild/framework/easyconfig/easyconfig.py

houndci-bot · 2018-09-12T08:39:01Z

test/framework/toy_build.py

            for modname in ['FFTW', 'OpenBLAS', 'ScaLAPACK']:
                regex = re.compile('load.*' + modname, re.M)
-                self.assertTrue(regex.search(toy_modtxt), "Pattern '%s' found in: %s" % (regex.pattern, toy_modtxt))
+                self.assertFalse(regex.search(toy_modtxt), "Pattern '%s' not found in: %s" % (regex.pattern, toy_modtxt))


line too long (121 > 120 characters)

ocaisa · 2018-09-12T10:52:23Z

@boegel Ready for re-review.

The one item left is that there is no way to force mapping a toolchain only to the top-level of the target, e.g., for an easyconfig with an iimpi toolchain --try-toolchain=foss,2018a will only map it to gompi and never to full foss (since gompi already has all the capabilities you need). I see this as correct default behaviour (users need to know nothing about subtoolchains, it all just works) but I can also a use case when you have a flat naming scheme. Where that is appropriate to add is up for discussion and I'd prefer to leave it to a different PR.

Ready for another pass

ocaisa · 2018-09-20T07:32:38Z

@boegel Would really like to see this included in 3.7.0

…ework into increase_try_scope

minor code cleanup

ocaisa · 2018-09-21T12:32:50Z

@boegel still does what it says on the tin

boegel · 2018-09-21T15:19:42Z

Going in, thanks a lot for your work on this @ocaisa!

Add support for including toolchains capabilities when creating toolc…

d5ba7d6

…hains hierarchy

ocaisa changed the base branch from master to develop July 25, 2018 12:28

houndci-bot reviewed Jul 25, 2018

View reviewed changes

Alan O'Cais added 4 commits July 25, 2018 14:29

Update comment in tests

cb4904d

No bare excepts

b0c5406

No bare excepts

5259b4a

Expand scope of get_toolchain_hierarchy without relying on build_options

c7ddafe

houndci-bot reviewed Jul 26, 2018

View reviewed changes

Address style errors

21b3a53

Alan O'Cais added 2 commits July 26, 2018 15:41

Add tests for hierarchy to heirarchy mapping

d3aba89

Be conservative with logic

46714a9

ocaisa changed the title ~~WIP: Increase try scope~~ WIP: Increase functionality of try_toolchain Jul 26, 2018

ocaisa commented Jul 26, 2018

View reviewed changes

Alan O'Cais added 4 commits July 26, 2018 16:19

Remove hardcoded GCCcore name

4a86c2b

Allowing mapping up from GCCcore to another compiler (in the case whe…

a5f1d36

…re GCCcore does not appear in the target)

Allowing mapping up from GCCcore to another compiler (in the case whe…

69080d3

…re GCCcore does not appear in the target)

Also grab binutils version built with GCCcore

7867ba9

houndci-bot reviewed Jul 30, 2018

View reviewed changes

Alan O'Cais added 3 commits July 30, 2018 13:16

Style fixes

444aea5

Add missing easyconfig to test suite

f256a32

Temporarily disable alternate method (to allow existing tests to pass)

619c9ff

houndci-bot reviewed Jul 30, 2018

View reviewed changes

Add tests for mapping an easyconfig to new hierarchy

9b20491

houndci-bot reviewed Jul 30, 2018

View reviewed changes

Miscellaneous bug fixes

ee1eb88

Address some of the comments

6c13972

boegel previously requested changes Sep 11, 2018

View reviewed changes

Alan O'Cais added 4 commits September 12, 2018 10:04

Address last of the comments

1f1b03d

Address newer comments

0965f9a

Fix comments

5377ff8

Fix more comments

61a0bcb

houndci-bot reviewed Sep 12, 2018

View reviewed changes

Alan O'Cais added 3 commits September 12, 2018 10:39

Appease the hound

14d80b7

Address more comments

4d8b503

Fix broken test

21a638c

This comment has been minimized.

Sign in to view

Use more general test method

07384d5

This comment has been minimized.

Sign in to view

Update tweak.py

5569c50

ocaisa mentioned this pull request Sep 20, 2018

(WIP) Automagic update dependencies #2136

Open

Alan O'Cais and others added 3 commits September 20, 2018 12:56

Merge branch 'develop' into increase_try_scope

b6942cc

Merge branch 'increase_try_scope' of github.com:ocaisa/easybuild-fram…

6c49a39

…ework into increase_try_scope

minor code cleanup

67b816e

boegel mentioned this pull request Sep 21, 2018

minor code cleanup ocaisa/easybuild-framework#37

Merged

Merge pull request #37 from boegel/increase_try_scope

6b627a0

minor code cleanup

boegel approved these changes Sep 21, 2018

View reviewed changes

boegel merged commit 44edbc5 into easybuilders:develop Sep 21, 2018

boegel modified the milestones: next release, 3.7.0 Sep 21, 2018

ocaisa deleted the increase_try_scope branch September 26, 2018 11:21

		source_tc_spec['compiler_family'], target_compiler_family)


		return minimal_matching_toolchain

		target_compiler_family = target_tc_spec['compiler_family']


		if not minimal_matching_toolchain:


		return can_map

		def match_minimum_tc_specs(source_tc_spec, target_tc_hierarchy):


		# TODO Replace the binutils version (if necessary)

		# TODO Determine the name of the modified easyconfig and dump it target_dir


		return resolve_dependencies(ec, modtool)

		def map_toolchain_hierarchies(source_toolchain, target_toolchain, modtool):


		return minimal_matching_toolchain

		def get_dep_tree_of_toolchain(toolchain_spec, modtool):

Increase functionality of try_toolchain #2539

Increase functionality of try_toolchain #2539

Conversation

ocaisa commented Jul 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocaisa commented Jul 26, 2018

ocaisa Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

ocaisa commented Sep 12, 2018

ocaisa commented Sep 20, 2018

ocaisa commented Sep 21, 2018

boegel commented Sep 21, 2018

ocaisa commented Jul 25, 2018 •

edited

Loading

ocaisa Jul 26, 2018 •

edited

Loading