Skip to content

change deprecated NVHPC toolchain classes to inherit from supported counterparts #5096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
boegel merged 10 commits into from
Jan 20, 2026
Merged

change deprecated NVHPC toolchain classes to inherit from supported counterparts #5096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e4e2923
change deprecated NVHPC compiler to just inherit the NVidiaCompilers …
lexming Jan 20, 2026
0df45d7
change deprecated NVHPCToolchain compiler to just inherit the NvidiaC…
lexming Jan 20, 2026
ddde426
simplify test_nvhpc_compatibility to just test compatibility downward…
lexming Jan 20, 2026
cd29eee
remove unused abc module from toolchains.nvhpc
lexming Jan 20, 2026
1b9bc7b
allow deprecated behaviour on tests handling toolchains
lexming Jan 20, 2026
a77cd7c
fix formatting in test.framework.docs
lexming Jan 20, 2026
3831601
remove deprecated NVHPC as compiler of NvidiaCompilers
lexming Jan 20, 2026
10c4ee6
add DEPRECATED attributed to deprecated toolchain classes
lexming Jan 20, 2026
7526dbd
filter out deprecated toolchains from toolchain list in documentation
lexming Jan 20, 2026
1e41a10
revert changes to tests listing toolchains in test.framework.options
lexming Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions easybuild/toolchains/compiler/nvhpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
# You should have received a copy of the GNU General Public License
# along with EasyBuild. If not, see <http://www.gnu.org/licenses/>.
##
"""Compatibility module such that compiler.nvhpc.NVHPC and compiler.compiler.nvidia_compilers.NvidiaCompilers
can be used interchangeably
"""
Compatibility module to allow using old compiler toolchain NVHPC
Deprecated in EasyBuild 5.2.0
"""

import abc
from easybuild.base import fancylogger
from easybuild.toolchains.compiler.nvidia_compilers import NvidiaCompilers

Expand All @@ -35,19 +35,14 @@
"easybuild.toolchains.compiler.nvidia_compilers in EasyBuild 5.2.0", '6.0')


# Former name used in EasyBuild until 5.2.0, now a DEPRECATED alias
class NVHPC(metaclass=abc.ABCMeta): # pylint: disable=too-few-public-methods
class NVHPC(NvidiaCompilers):
"""DEPRECATED alias for NvidiaCompilers."""
def __new__(cls, *args, **kwargs):
if cls is NVHPC:
inst = NvidiaCompilers(*args, **kwargs)
inst.log.deprecated(
"easybuild.toolchains.compiler.nvhpc was replaced by "
"easybuild.toolchains.compiler.nvidia_compilers in EasyBuild 5.2.0", '6.0')
return inst
return super().__new__(cls)

DEPRECATED = True

NVHPC.register(NvidiaCompilers)
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# TODO EasyBuild 6.0: Remove NVHPC name from NvidiaCompilers.COMPILER_MODULE_NAME
self.log.deprecated(
"easybuild.toolchains.compiler.nvhpc was replaced by "
"easybuild.toolchains.compiler.nvidia_compilers in EasyBuild 5.2.0", '6.0'
)
3 changes: 1 addition & 2 deletions easybuild/toolchains/compiler/nvidia_compilers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
class NvidiaCompilers(Compiler):
"NVHPC compiler class"

# TODO EasyBuild 6.0: Remove NVHPC name
COMPILER_MODULE_NAME = ['nvidia-compilers', 'NVHPC']
COMPILER_MODULE_NAME = ['nvidia-compilers']

COMPILER_FAMILY = TC_CONSTANT_NVHPC

Expand Down
14 changes: 5 additions & 9 deletions easybuild/toolchains/nvhpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
* Andreas Herten (Forschungszentrum Juelich)
* Alex Domingo (Vrije Universiteit Brussel)
"""
import abc
from easybuild.toolchains.gcccore import GCCcore
from easybuild.toolchains.linalg.nvblas import NVBLAS
from easybuild.toolchains.linalg.nvscalapack import NVScaLAPACK
Expand All @@ -48,14 +47,11 @@ class NVHPC(NvidiaCompilersToolchain, NVHPCX, NVBLAS, NVScaLAPACK):
SUBTOOLCHAIN = [NvidiaCompilersToolchain.NAME, GCCcore.NAME, SYSTEM_TOOLCHAIN_NAME]


class NVHPCToolchain(metaclass=abc.ABCMeta): # pylint: disable=too-few-public-methods
class NVHPCToolchain(NvidiaCompilersToolchain):
"""DEPRECATED alias for NvidiaCompilersToolchain."""
def __new__(cls, *args, **kwargs):
if cls is NVHPCToolchain:
inst = NvidiaCompilersToolchain(*args, **kwargs)
inst.log.deprecated("NVHPCToolchain was replaced by NvidiaCompilersToolchain in EasyBuild 5.2.0", '6.0')
return inst
return super().__new__(cls)
DEPRECATED = True

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

NVHPCToolchain.register(NvidiaCompilersToolchain)
self.log.deprecated("NVHPCToolchain was replaced by NvidiaCompilersToolchain in EasyBuild 5.2.0", '6.0')
6 changes: 5 additions & 1 deletion easybuild/tools/docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,11 @@ def list_toolchains(output_format=FORMAT_TXT):

# start with dict that maps toolchain name to corresponding subclass of Toolchain
# filter deprecated 'dummy' toolchain
tcs = {tc.NAME: tc for tc in all_tcs}
tcs = {
tc.NAME: tc
for tc in all_tcs
if not hasattr(tc, "DEPRECATED")
}

for tcname in sorted(tcs):
tcc = tcs[tcname]
Expand Down
17 changes: 6 additions & 11 deletions test/framework/toolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3390,7 +3390,6 @@ def test_get_flag(self):
def test_nvhpc_compatibility(self):
"""Test that software using EasyBuild before 5.2.0 continues working
the compiler.nvhpc toolchain being renamed to NvidiaCompilers"""
from easybuild.toolchains.nvompi import Nvompi
from easybuild.toolchains.compiler.nvidia_compilers import NvidiaCompilers
from easybuild.toolchains.nvhpc import NvidiaCompilersToolchain, NVHPCToolchain

Expand All @@ -3399,29 +3398,25 @@ def test_nvhpc_compatibility(self):
from easybuild.toolchains.compiler.nvhpc import NVHPC
self.assertIn("nvhpc was replaced by easybuild.toolchains.compiler.nvidia_compilers", self.get_stderr())

# load NVidiaCompilers, deprecated NVHPC corresponds to it
tc = NvidiaCompilers(name='NvidiaCompilers', version='2024a') # Common usage
# Might be checked by pre-5.2.0 users
self.assertIsInstance(tc, NVHPC)
Copy link
Contributor

@Flamefire Flamefire Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove this? This was one of the breaking changes: Prior users relied on checking if isinstance(tc, NVHPC) which was broken by removing NVHPC.

Copy link
Contributor Author

@lexming lexming Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it does not matter. We care that the deprecated toolchain is still there, can be loaded and provides the new one. The opposite is irrelevant, nobody should load the new toolchain and expect to get the deprecated one.

Copy link
Contributor

@Flamefire Flamefire Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nobody should load the new toolchain and expect to get the deprecated one.

But people use the old toolchain already by importing the old one not the new one. This change here (and in 5.2.0) breaks existing usage, e.g. in hooks:

from easybuild.toolchains.compilers import NVHPC

def some_hook(ec):
  tc = ec.get_toolchain()
  if isinstance(tc, NVHPC):
    ec['cuda_compute_capabilities'] = '8.0'

Because we changed the name to be Nvidia-compilers this does not work anymore and needs to be adapted to if isinstance(tc, NVHPC, NvidiaCompilers): to work with 5.1 and 5.2

Hence my ABC-Meta trick to allow the existing code to work even with the 5.2.0 change

Copy link
Contributor Author

@lexming lexming Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not break anything. That check is catching the old toolchain, and continues to be able to catch the old toolchain.

You are right that this code will not catch the new toolchain, and that is correct IMO. If you want to catch the new stuff, you need to update your hooks and be actively aware that your hooks are catching the new stuff. The old and new toolchains are very much different, so the code behind that check is not guaranteed to work with the new stuff at all in any case.

Copy link
Contributor

@Flamefire Flamefire Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old and new toolchains are very much different, so the code behind that check is not guaranteed to work with the new stuff at all in any case.

Is this true? Because #4927 has commit f1761f8

rename NVHPC compiler toolchain to nvidia-compilers

So I understood this as a rename of compiler.NVHPC to compiler.NvidiaCompilers with no (real) differences.

Note that I'm not referring to toolchains.NVHPC but to the previously existing "compiler"

Copy link
Contributor Author

@lexming lexming Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is true, we started with just a rename. But that does not mean that we will keep backward compatibility with old compiler.NVHPC at all levels. And things have already diverged, not in structure (as you mention compiler.NvidiaCompilers holds the same spot as former compiler.NVHPC) but in functionality.

The new easyblocks are rather different, attributes and methods have changed. So we cannot guarantee that a hook for the old NVHPC will continue to work with the new one at any level. Therefore, it is risky to have hooks running inadvertently with the new code. Even if that isinstance check would work, code underneath can potentially fail without an update.

This does not mean that we don't care about backward compatibility at all. We already made sure that existing easyconfigs for the old NVHPC continue to work in the same way with the new nvidia-compilers/NVHPC. And we also enabled loading the old NVHPC so that it can still be used by custom easyblocks or custom easyconfigs.

Copy link
Contributor

@Flamefire Flamefire Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we cannot guarantee that a hook for the old NVHPC will continue to work with the new one at any level.

Then I'd rather not have compiler.NVHPC. With the current change the code above seems to work but is never triggered. That is even worse than before. Better always wrong, than sometimes silently wrong.

What I was aiming for was a "best effort".
I don't know what the differences between the 2 toolchain classes (easyblocks?) are now. My goal was that working with instances of either works the same as much as possible, both when the user had a custom derived easyblock and when the user got an instance of (now) the new one.


# load deprecated NVHPC, NvidiaCompilers corresponds to it
# load deprecated NVHPC and check it corresponds to NvidiaCompilers
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
tc = NVHPC(name='NVHPC', version='2024a') # Might be used by pre-5.2.0 users
self.assertIn("nvhpc was replaced by easybuild.toolchains.compiler.nvidia_compilers", self.get_stderr())
self.assertIsInstance(tc, NvidiaCompilers)

# load deprecated NVHPCToolchain, it corresponds to NvidiaCompilersToolchain
# load deprecated NVHPCToolchain and check it corresponds to NvidiaCompilersToolchain
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
tc = NVHPCToolchain(name='NVHPC', version='2024a') # Might be used by pre-5.2.0 users
self.assertIn("NVHPCToolchain was replaced by NvidiaCompilersToolchain", self.get_stderr())
self.assertIsInstance(tc, NvidiaCompilersToolchain)

from easybuild.toolchains.nvompi import Nvompi
tc = Nvompi(version='2024a') # Common usage
self.assertIsInstance(tc, NvidiaCompilers)
self.assertIsInstance(tc, NvidiaCompilersToolchain)
self.assertIsInstance(tc, NVHPCToolchain)
# compiler toolchain NVHPC is deprecated in 5.2.0, but might still be checked by existing code
self.assertIsInstance(tc, NVHPC)
# Nvompi migrated to new NvidiaCompiler in v5.2.0, compiler toolchain NVHPC deprecated
self.assertNotIsInstance(tc, NVHPCToolchain)
self.assertNotIsInstance(tc, NVHPC)


def suite(loader=None):
Expand Down