Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script #1229

damianam · 2017-08-15T15:12:38Z

(ie: do not add LD_LIBRARY_PATH, LIBRARY_PATH and CPATH)

Otherwise error like this appear:

$PREFIX/VTune/2017_update4/vtune_amplifier_xe/lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by XXX)

…ript (ie: do not add LD_LIBRARY_PATH, LIBRARY_PATH and CPATH)

boegel · 2017-08-16T07:49:39Z

easybuild/easyblocks/v/vtune.py


        if self.cfg['m32']:
            guesses.update({
                'PATH': [os.path.join(self.subdir, 'bin32')],
-                'LD_LIBRARY_PATH': [os.path.join(self.subdir, 'lib32')],
-                'LIBRARY_PATH': [os.path.join(self.subdir, 'lib32')],
            })


since this is now a single update, let's change this to:

guesses['PATH'] = [os.path.join(self.subdir, 'bin32')]

same below

boegel · 2017-08-16T07:52:16Z

easybuild/easyblocks/v/vtune.py

            'MANPATH': [os.path.join(self.subdir, 'man')],
        })

+        for ev in do_not_set:


do_not_set is not used anywhere else, so no need to introduce a variable

also, clarify with a comment why this is being done, and include (debug) log messages for kicking those out:

# make sure $CPATH, $LD_LIBRARY_PATH and $LIBRARY_PATH are not updated in generated module file, # because that leads to problem when the libraries included with VTune are being picked up for key in ['CPATH', 'LD_LIBRARY_PATH', 'LIBRARY_PATH']: if key in guesses: self.log.debug("Purposely ot updating $%s in VTune module file", key) del guesses[key]

@damianam ping?

@boegel pong back at ya'!

damianam · 2017-09-18T14:30:13Z

I think it makes sense to define an IntelTools that inherits from IntelBase, and used as a base for EB_Advisor, EB_Vtune and EB_Inspector. Thoughts?

… guessing and sanity checking, pushed up to intelbase.py

boegel

Some small remarks, nice cleanup on the side, yaay!

boegel · 2017-12-07T16:34:08Z

easybuild/easyblocks/a/advisor.py

@@ -46,31 +46,12 @@ def __init__(self, *args, **kwargs):
        else:
            self.base_path = 'advisor'

+    def make_module_req_guess(self):
+        """Find reasonable paths for Advisor"""
+        return _make_module_req_guess_tools(self)


self.make_module_req_guess_tools()

boegel · 2017-12-07T16:34:22Z

easybuild/easyblocks/a/advisor.py

-        }
-
+        binaries = ['advixe-cl', 'advixe-feedback', 'advixe-gui', 'advixe-runss', 'advixe-runtrc', 'advixe-runtc']
+        custom_paths = _get_custom_paths_tools(self,binaries)


custom_paths = self.get_custom_paths_tools(binaries)

boegel · 2017-12-07T16:40:27Z

easybuild/easyblocks/i/inspector.py

@@ -44,7 +44,8 @@ def __init__(self, *args, **kwargs):

        # recent versions of Inspector are installed to a subdirectory
        self.subdir = ''
-        if LooseVersion(self.version) >= LooseVersion('2013_update7') and LooseVersion(self.version) < LooseVersion('2017'):
+        if LooseVersion(self.version) >= LooseVersion('2013_update7') and \
+           LooseVersion(self.version) < LooseVersion('2017'):


Ugh, not a big fan of the break via \ (we don't do this anywhere else either).

If you want to shorten this, I'd suggest:

loosever = LooseVersion(self.version) if loosever >= LooseVersion('2013_update7') and loosever < LooseVersion('2017'):

Ok. I've just done it like that because the vtune easyblock had that break in that point.

boegel · 2017-12-07T16:40:45Z

easybuild/easyblocks/i/inspector.py

-
-        return guesses
+        """Find reasonable paths for Inspector"""
+        return _get_guesses_tools(self)


self.get_guesses_tools()

boegel · 2017-12-07T16:41:15Z

easybuild/easyblocks/i/inspector.py

-            'files': [os.path.join(self.subdir, f) for f in files],
-            'dirs': [os.path.join(self.subdir, d) for d in dirs],
-        }
+        custom_paths = _get_custom_paths_tools(self, binaries)


custom_paths = self.get_custom_paths_tools(binaries)

boegel · 2017-12-07T16:42:15Z

easybuild/easyblocks/v/vtune.py

        self.subdir = ''
-        if LooseVersion(self.version) >= LooseVersion('2013_update12'):
+        if LooseVersion(self.version) >= LooseVersion('2013_update12') and \
+           LooseVersion(self.version) < LooseVersion('2018'):


loosever = LooseVersion(self.version) if loosever >= LooseVersion('2013_update12') and loosever < LooseVersion('2018'): self.subdir = 'vtune_amplifier_xe' elif loosever >= LooseVersion('2018'): self.subdir = 'vtune_amplifier'

boegel · 2017-12-07T16:42:27Z

easybuild/easyblocks/v/vtune.py

-
-        return guesses
+        """Find reasonable paths for VTune"""
+        return _get_guesses_tools(self)


self.get_guesses_tools()

boegel · 2017-12-07T16:42:46Z

easybuild/easyblocks/v/vtune.py

-            'files': [os.path.join(self.subdir, f) for f in files],
-            'dirs': [os.path.join(self.subdir, d) for d in dirs],
-        }
+        custom_paths = _get_custom_paths_tools(self, binaries)


custom_paths = self.get_custom_paths_tools(binaries)

boegel · 2017-12-07T16:44:12Z

easybuild/easyblocks/generic/intelbase.py

@@ -111,6 +112,42 @@ def __init__(self, *args, **kwargs):

        self.install_components = None

+    def _get_guesses_tools(self):


@damianam I wouldn't make these hidden/private, since that is an indication these are/should only be used inside IntelBase, which is the opposite of what you're doing...

So, use def get_guesses_tools(self):

Yep, I was a bit confused on the exact meaning of the underscore. I mean, I want these methods to be called exclusively by classes that inherited from IntelBase. But I think python don't have that kind of distinction.

The leading underscore is only a convention too, it doesn't actually prevent you to do anything, it's just an indication that you shouldn't.

I don't think there's a way to restrict these methods to only be called by specific easyblocks, at least not without making an IntelTools that sits on top of IntelBase...

boegel · 2017-12-07T16:44:48Z

easybuild/easyblocks/generic/intelbase.py

+
+        return guesses
+
+    def _get_custom_paths_tools(self, binaries):


same here, use def _get_custom_paths_tools(self, binaries):

boegel · 2017-12-07T18:54:24Z

Tested with all existing VTune, Advisor and Inspector easyconfigs, lgtm so going in...

Thanks @damianam!

Updated VTune easyblock to behave like sourcing the amplxe-vars.sh sc…

0c39489

…ript (ie: do not add LD_LIBRARY_PATH, LIBRARY_PATH and CPATH)

boegel requested changes Aug 16, 2017

View reviewed changes

boegel added this to the 3.4.0 milestone Aug 16, 2017

Added the same fix for Advisor and Inspector

90dac0c

boegel modified the milestones: 3.5.0, 3.4.0 Sep 4, 2017

Cleanup of advisor, vtune and inspector easyblocks

fac5316

damianam added the bug report label Sep 18, 2017

damianam modified the milestones: 3.5.0, 3.4.1 Sep 18, 2017

Damian Alvarez added 2 commits September 18, 2017 18:17

Added support for VTune 2018

89c5537

Fixed an oopsie

84b6718

boegel modified the milestones: 3.4.1, 3.5.0 Oct 11, 2017

boegel modified the milestones: 3.5.0, next release Dec 6, 2017

Damian Alvarez added 3 commits December 7, 2017 15:21

Fixed misssing import

8959723

Common code in advisor.py, inspector.py and vtune.py, related to path…

292302d

… guessing and sanity checking, pushed up to intelbase.py

Unnecessary import deleted

e02c402

boegel changed the title ~~Updated VTune easyblock to behave like sourcing the amplxe-vars.sh sc…~~ Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script Dec 7, 2017

boegel requested changes Dec 7, 2017

View reviewed changes

easybuilders deleted a comment from boegelbot Dec 7, 2017

Fixed a couple of mistakes

a490dc0

boegel approved these changes Dec 7, 2017

View reviewed changes

boegel merged commit 5455a25 into easybuilders:develop Dec 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script #1229

Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script #1229

damianam commented Aug 15, 2017 •

edited by boegel

Loading

boegel Aug 16, 2017

boegel Aug 16, 2017

boegel Sep 4, 2017

damianam Sep 18, 2017

damianam commented Sep 18, 2017

boegel left a comment

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

damianam Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

damianam Dec 7, 2017

boegel Dec 7, 2017

boegel Dec 7, 2017

boegel commented Dec 7, 2017

		@@ -111,6 +112,42 @@ def __init__(self, args, *kwargs):

		self.install_components = None

		def _get_guesses_tools(self):

Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script #1229

Updated VTune easyblock to behave like sourcing the amplxe-vars.sh script #1229

Conversation

damianam commented Aug 15, 2017 • edited by boegel 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

damianam commented Sep 18, 2017

boegel left a 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

boegel commented Dec 7, 2017

damianam commented Aug 15, 2017 •

edited by boegel

Loading