Skip to content
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.

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

Parsing compiler name & version from Spack environment to include in the performance log file #262

Merged
merged 25 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3d72c50
Added compiler name & version to the performance log file
kaanolgu Jan 31, 2024
323ae11
Moving all changes to utils.py
kaanolgu Feb 2, 2024
b4f3658
spack_spec keys and values are printed apart from the compiler name a…
kaanolgu Feb 13, 2024
8ba7561
[NEW] Dictionary for spack_spec
kaanolgu Feb 26, 2024
b79f781
[NEW] MPI output
kaanolgu Feb 29, 2024
7b5e51e
Attempt to solve CI/test failed error
kaanolgu Feb 29, 2024
7a02712
Merge branch 'main' into ko/spack-spec-parser
kaanolgu Feb 29, 2024
f696cd5
[NEW] Concretized multiple spac_spec all in one dictionary
kaanolgu Mar 10, 2024
dd066c1
Merge branch 'ko/spack-spec-parser' of https://github.com/ukri-excali…
kaanolgu Mar 11, 2024
51c5713
[FIX] CI failing
kaanolgu Mar 11, 2024
49f2286
* MPI is moved into dictionary of each spec
kaanolgu Mar 21, 2024
4850d38
Merge branch 'main' into ko/spack-spec-parser
kaanolgu Mar 22, 2024
aed6bc2
[FIX] MPI to be string if present
kaanolgu Mar 25, 2024
5ac953f
Merge branch 'ko/spack-spec-parser' of github.com:ukri-excalibur/exca…
kaanolgu Mar 25, 2024
878aa5e
Update expected fields with unpacked names
tkoskela Mar 25, 2024
493c7bf
Install with -e
tkoskela Mar 25, 2024
3559fec
Minor formatting fixes.
pineapple-cat Mar 27, 2024
57767c7
Modified new key column insertion and added recursive flattening of r…
pineapple-cat Mar 27, 2024
c17b12e
Merge branch 'main' into ko/spack-spec-parser
pineapple-cat Mar 27, 2024
7c871e5
Fixed key columns return bug.
pineapple-cat Apr 9, 2024
9fbc4f1
Merge branch 'main' into ko/spack-spec-parser
kaanolgu Apr 9, 2024
cbcaeeb
Fix bug with unpacking nested dicts in column names - there might be …
ilectra Apr 10, 2024
2e46df8
Added unit test for key column unpacking.
pineapple-cat Apr 10, 2024
1177f71
Merge branch 'main' into ko/spack-spec-parser
pineapple-cat Apr 10, 2024
1272d8c
Debug unit test for recursive dictionary unpacking.
ilectra Apr 11, 2024
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
38 changes: 35 additions & 3 deletions benchmarks/modules/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
from reframe.core.exceptions import BuildSystemError
from reframe.core.logging import getlogger
from reframe.utility.osext import run_command


import reframe.utility.osext as osext
import reframe.utility.sanity as sn
SYSFILE = 'systems/sysinfo.json' # interpreted relative to jupyter root

def get_jupyter_root():
Expand Down Expand Up @@ -243,6 +243,10 @@ def identify_build_environment(current_partition):
class SpackTest(rfm.RegressionTest):
build_system = 'Spack'
spack_spec = variable(str, value='', loggable=True)
spack_variants = variable(str, value='', loggable=True)
compiler_version = variable(str, value='', loggable=True)
compiler_name = variable(str, value='', loggable=True)
spack_mpi = variable(str, value='', loggable=True)

@run_before('compile')
def setup_spack_environment(self):
Expand All @@ -267,11 +271,38 @@ def setup_spack_environment(self):
f'(cd {cp_dir}; find . \( -name "spack.yaml" -o -name "compilers.yaml" -o -name "packages.yaml" \) -print0 | xargs -0 tar cf - | tar -C {dest} -xvf -)',
f'spack -e {self.build_system.environment} config add "config:install_tree:root:{env_dir}/opt"',
]

# else f"{d[key].value}" if isinstance(d[key].value, bool) for boolean with "True" instead of True
cmd_spack_variants = 'from spack import environment; d = environment.active_environment().spec_lists["specs"].specs[0].variants.dict; keys = list(d.keys());values = {key: ([d[key].value[i] for i in range(len(d[key].value))] if isinstance(d[key].value, tuple) else d[key].value) for key in keys};print(values)'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the whole variant string broken down into keys automatically or as one string?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually.... If this contains the compiler name and version as well, do we still need the separate variables and commands to fish them out? Can we let the post-processing unpack it into separate columns instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it would be better to keep variants and compiler information separate since compiler name and version is always available but some applications won't even have a variant at all maybe and spack_spec as a string is available in the columns as a whole anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In all these commands, you extract only the first package from the spec with print(environment.active_environment().spec_lists["specs"].specs[0] . There should be a dictionary for each package instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In all these commands, you extract only the first package from the spec with print(environment.active_environment().spec_lists["specs"].specs[0 ] . There should be a dictionary for each package instead.

it is a bit confusing. When you execute the following command :

reframe -c benchmarks/apps/babelstream -r --tag cuda --system=isambard-phase3:ampere --setvar=num_cpus_per_task=40 -S build_locally=false -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=80'

The specs prints out ['babelstream%[email protected] +cuda cuda_arch=80'] so specs[0] is needed to get the whole spack_spec

If you try something like :

...  -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=80,babelstream%[email protected] +cuda cuda_arch=72 '
...  -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=80 babelstream%[email protected] +cuda cuda_arch=72 '
...  -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=80' -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=72 '
...  -Sspack_spec='babelstream%[email protected] +cuda cuda_arch=80','babelstream%[email protected] +cuda cuda_arch=72 '
...  -Sspack_spec=['babelstream%[email protected] +cuda cuda_arch=80','babelstream%[email protected] +cuda cuda_arch=72 ']

They won't work, so afaik ReFrame allows multiple runs with tag but with only a single spack_spec to be specified for each run, that's why I am using specs[0]. If it is possible to run multiple spack_specs at the same time then it would be great to learn and change the PR accordingly since it would be handy for our tests too 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's not multiple specs, but multiple packages inside the one spec, see eg.

spack_spec = 'intel-mkl intel-mpi'

Do I get this right @giordano?

Copy link
Member

Choose a reason for hiding this comment

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

Do I get this right @giordano?

Almost, they are two distinct top-level specs in one environment. But the point is that, as I had already explained in the second point of the caveats section of #251 (comment), you can't assume there's exactly 1 top-level spec in the environment, which is what you're doing when you do specs[0].

ilectra marked this conversation as resolved.
Show resolved Hide resolved
cmd_spec_mpi = 'from spack import environment;print(environment.active_environment().spec_lists["specs"].specs[0]["mpi"]) if "mpi" in environment.active_environment().spec_lists["specs"].specs[0] else "" '
cmd_compiler_name = 'from spack import environment; print(environment.active_environment().spec_lists["specs"].specs[0].compiler.name)'
# Although we could provide several versions of compiler, reframe only executes with the first version
cmd_compiler_version = 'from spack import environment;environment.active_environment().spec_lists["specs"].specs[0].compiler.versions[0]'
self.postrun_cmds.append(f'echo "compiler_name: $(spack -e {self.build_system.environment} python -c \'{cmd_compiler_name}\')"')
self.postrun_cmds.append(f'echo "compiler_version: $(spack -e {self.build_system.environment} python -c \'{cmd_compiler_version}\')"')
self.postrun_cmds.append(f'echo "spack_spec_variants: $(spack -e {self.build_system.environment} python -c \'{cmd_spack_variants}\')"')
self.postrun_cmds.append(f'echo "MPI: $(spack -e {self.build_system.environment} python -c \'{cmd_spec_mpi}\')"')

# Keep the `spack.lock` file in the output directory so that the Spack
# environment can be faithfully reproduced later.
self.keep_files.append(os.path.realpath(os.path.join(self.build_system.environment, 'spack.lock')))

@run_after('run')
def get_compiler_name(self):
with osext.change_dir(self.stagedir):
self.compiler_name = sn.extractsingle(r'compiler_name: \s*(\S+)', self.stdout, 1).evaluate()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to do this as a two step process? (write out the information to stdout before compilation and then read it from stdout after running) Is there a way to do it in one go, at the right time for ReFrame?

@run_after('run')
def get_compiler_version(self):
with osext.change_dir(self.stagedir):
self.compiler_version = sn.extractsingle(r'compiler_version: \s*(.*)', self.stdout, 1).evaluate()
@run_after('run')
def get_full_variants(self):
with osext.change_dir(self.stagedir):
self.spack_variants = sn.extractsingle(r'spack_spec_variants: \s*(.*)', self.stdout, 1).evaluate()
@run_after('run')
def get_spec_mpi(self):
with osext.change_dir(self.stagedir):
self.spack_mpi = sn.extractsingle(r'MPI: \s*(.*)', self.stdout, 1).evaluate()

@run_before('compile')
def setup_build_system(self):
# The `self.spack_spec` attribute is the user-facing and loggable
Expand Down Expand Up @@ -301,6 +332,7 @@ def setup_build_job_num_cpus(self):
self.build_job.num_cpus_per_task = min(16, self.current_partition.processor.num_cpus)



if __name__ == '__main__':

#v = get_sysinfo(sys.argv[-1])
Expand Down
4 changes: 4 additions & 0 deletions benchmarks/reframe_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,10 @@ def spack_root_to_path():
'%(check_environ)s|'
'%(check_extra_resources)s|'
'%(check_env_vars)s|'
'%(check_compiler_name)s|'
'%(check_compiler_version)s|'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you also want the variant fished out of the spack_spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, seems like not needed anymore since tags from ReFrame has the programming model variant, and compiler name alongside with version is provided with this PR. In the new Spack package we have made some changes to make sure each programming model has its own dedicated flags. So, it is tough to have a one solution fits all for the rest of the variants for Babelstream and also for other apps too since naming conventions are all different.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then maybe we should export the whole variant? And users of each app can choose to use it and unpack it or not.

'%(check_spack_variants)s|'
'%(check_spack_mpi)s|'
'%(check_tags)s'
),
'format_perfvars': (
Expand Down
6 changes: 5 additions & 1 deletion post-processing/test_post_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,15 @@ def test_read_perflog(run_sombrero):
# get dataframe from complete perflog
df = log_hand.read_perflog(sombrero_log_path)


EXPECTED_FIELDS = ["job_completion_time", "version", "info", "jobid", "num_tasks",
"num_cpus_per_task", "num_tasks_per_node", "num_gpus_per_node",
"flops_value", "flops_unit", "flops_ref", "flops_lower_thres",
"flops_upper_thres", "spack_spec", "test_name", "tasks", "cpus_per_task",
"system", "partition", "environ", "OMP_NUM_THREADS", "tags"]
"system", "partition", "environ", "OMP_NUM_THREADS", "spack_variants",
"compiler_name", "compiler_version", "spack_mpi", "tags"]



# check example perflog file is read appropriately
# check all expected columns are present
Expand Down