targets: print a warning message before downgrading (#13513)
* Make package preferences a soft failure for targets, instead of a hard failure. * Added unit tests for preferences expressed via packages.yaml
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 Todd Gamblin
						Todd Gamblin
					
				
			
			
				
	
			
			
			 Todd Gamblin
						Todd Gamblin
					
				
			
						parent
						
							43b18dada4
						
					
				
				
					commit
					42b8355269
				
			| @@ -281,6 +281,24 @@ def concretize_architecture(self, spec): | |||||||
|             else: |             else: | ||||||
|                 # To get default platform, consider package prefs |                 # To get default platform, consider package prefs | ||||||
|                 if PackagePrefs.has_preferred_targets(spec.name): |                 if PackagePrefs.has_preferred_targets(spec.name): | ||||||
|  |                     new_target = self.target_from_package_preferences(spec) | ||||||
|  |                 else: | ||||||
|  |                     new_target = new_plat.target('default_target') | ||||||
|  |  | ||||||
|  |         # Construct new architecture, compute whether spec changed | ||||||
|  |         arch_spec = (str(new_plat), str(new_os), str(new_target)) | ||||||
|  |         new_arch = spack.spec.ArchSpec(arch_spec) | ||||||
|  |         spec_changed = new_arch != spec.architecture | ||||||
|  |         spec.architecture = new_arch | ||||||
|  |         return spec_changed | ||||||
|  |  | ||||||
|  |     def target_from_package_preferences(self, spec): | ||||||
|  |         """Returns the preferred target from the package preferences if | ||||||
|  |         there's any. | ||||||
|  |  | ||||||
|  |         Args: | ||||||
|  |             spec: abstract spec to be concretized | ||||||
|  |         """ | ||||||
|         target_prefs = PackagePrefs(spec.name, 'target') |         target_prefs = PackagePrefs(spec.name, 'target') | ||||||
|         target_specs = [spack.spec.Spec('target=%s' % tname) |         target_specs = [spack.spec.Spec('target=%s' % tname) | ||||||
|                         for tname in cpu.targets] |                         for tname in cpu.targets] | ||||||
| @@ -298,17 +316,8 @@ def tspec_filter(s): | |||||||
|         # Sort filtered targets by package prefs |         # Sort filtered targets by package prefs | ||||||
|         target_specs = list(filter(tspec_filter, target_specs)) |         target_specs = list(filter(tspec_filter, target_specs)) | ||||||
|         target_specs.sort(key=target_prefs) |         target_specs.sort(key=target_prefs) | ||||||
|  |  | ||||||
|         new_target = target_specs[0].architecture.target |         new_target = target_specs[0].architecture.target | ||||||
|                 else: |         return new_target | ||||||
|                     new_target = new_plat.target('default_target') |  | ||||||
|  |  | ||||||
|         # Construct new architecture, compute whether spec changed |  | ||||||
|         arch_spec = (str(new_plat), str(new_os), str(new_target)) |  | ||||||
|         new_arch = spack.spec.ArchSpec(arch_spec) |  | ||||||
|         spec_changed = new_arch != spec.architecture |  | ||||||
|         spec.architecture = new_arch |  | ||||||
|         return spec_changed |  | ||||||
|  |  | ||||||
|     def concretize_variants(self, spec): |     def concretize_variants(self, spec): | ||||||
|         """If the spec already has variants filled in, return.  Otherwise, add |         """If the spec already has variants filled in, return.  Otherwise, add | ||||||
| @@ -526,7 +535,12 @@ def _adjust_target(self, spec): | |||||||
|         current_platform = spack.architecture.get_platform( |         current_platform = spack.architecture.get_platform( | ||||||
|             spec.architecture.platform |             spec.architecture.platform | ||||||
|         ) |         ) | ||||||
|         if current_target != current_platform.target('default_target') or \ |  | ||||||
|  |         default_target = current_platform.target('default_target') | ||||||
|  |         if PackagePrefs.has_preferred_targets(spec.name): | ||||||
|  |             default_target = self.target_from_package_preferences(spec) | ||||||
|  |  | ||||||
|  |         if current_target != default_target or \ | ||||||
|             (self.abstract_spec.architecture is not None and |             (self.abstract_spec.architecture is not None and | ||||||
|              self.abstract_spec.architecture.target is not None): |              self.abstract_spec.architecture.target is not None): | ||||||
|             return False |             return False | ||||||
| @@ -544,6 +558,11 @@ def _adjust_target(self, spec): | |||||||
|                     continue |                     continue | ||||||
|  |  | ||||||
|                 if candidate is not None: |                 if candidate is not None: | ||||||
|  |                     msg = ('{0.name}@{0.version} cannot build optimized ' | ||||||
|  |                            'binaries for "{1}". Using best target possible: ' | ||||||
|  |                            '"{2}"') | ||||||
|  |                     msg = msg.format(spec.compiler, current_target, candidate) | ||||||
|  |                     tty.warn(msg) | ||||||
|                     spec.architecture.target = candidate |                     spec.architecture.target = candidate | ||||||
|                     return True |                     return True | ||||||
|             else: |             else: | ||||||
|   | |||||||
| @@ -11,6 +11,7 @@ | |||||||
| import spack.repo | import spack.repo | ||||||
|  |  | ||||||
| from spack.concretize import find_spec, NoValidVersionError | from spack.concretize import find_spec, NoValidVersionError | ||||||
|  | from spack.package_prefs import PackagePrefs | ||||||
| from spack.spec import Spec, CompilerSpec | from spack.spec import Spec, CompilerSpec | ||||||
| from spack.spec import ConflictsInSpecError, SpecError | from spack.spec import ConflictsInSpecError, SpecError | ||||||
| from spack.version import ver | from spack.version import ver | ||||||
| @@ -83,13 +84,25 @@ def spec(request): | |||||||
|  |  | ||||||
|  |  | ||||||
| @pytest.fixture(params=[ | @pytest.fixture(params=[ | ||||||
|     'haswell', 'broadwell', 'skylake', 'icelake' |     # Mocking the host detection | ||||||
|  |     'haswell', 'broadwell', 'skylake', 'icelake', | ||||||
|  |     # Using preferred targets from packages.yaml | ||||||
|  |     'icelake-preference', 'cannonlake-preference' | ||||||
| ]) | ]) | ||||||
| def current_host(request, monkeypatch): | def current_host(request, monkeypatch): | ||||||
|     target = llnl.util.cpu.targets[request.param] |     # is_preference is not empty if we want to supply the | ||||||
|  |     # preferred target via packages.yaml | ||||||
|  |     cpu, _, is_preference = request.param.partition('-') | ||||||
|  |     target = llnl.util.cpu.targets[cpu] | ||||||
|  |     if not is_preference: | ||||||
|         monkeypatch.setattr(llnl.util.cpu, 'host', lambda: target) |         monkeypatch.setattr(llnl.util.cpu, 'host', lambda: target) | ||||||
|     monkeypatch.setattr(spack.platforms.test.Test, 'default', request.param) |         monkeypatch.setattr(spack.platforms.test.Test, 'default', cpu) | ||||||
|     return target |         yield target | ||||||
|  |     else: | ||||||
|  |         # There's a cache that needs to be cleared for unit tests | ||||||
|  |         PackagePrefs._packages_config_cache = None | ||||||
|  |         with spack.config.override('packages:all', {'target': [cpu]}): | ||||||
|  |             yield target | ||||||
|  |  | ||||||
|  |  | ||||||
| @pytest.mark.usefixtures('config', 'mock_packages') | @pytest.mark.usefixtures('config', 'mock_packages') | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user