MetOffice / MetOffice/lfric_core

Profiler logic tests definition not boolean logic

Open Beginner friendly
#444 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Fortran
Stars
26
Forks
73
Avg merge
1d 8h
Merged PRs (30d)
4

Description

### Version

main

### Are there any linked Issues or Pull Requests?

_No response_

### What happened?

Throughout lfric_apps boolean logic is used for specifying profiler:

https://github.com/MetOffice/lfric_apps/blob/main/rose-stem/site/meto/common/suite_config_azspice.cylc#L34
```
[[AZSPICE_BUILD]]
[[[environment]]]
USE_LEGACY_TIMER=true
USE_TIMING_WRAPPER=true
[[[directives]]]
--gres=tmp:1024
--export=NONE
```

And yet the logic to raise an error when multiple profilers is used test for definition not boolean logic

https://github.com/MetOffice/lfric_core/blob/main/infrastructure/build/lfric.mk#L90
```
ifdef USE_VERNIER
export PRE_PROCESS_MACROS += VERNIER
endif

ifdef USE_LEGACY_TIMER
export PRE_PROCESS_MACROS += LEGACY_TIMER
endif

ifdef USE_TIMING_WRAPPER
export PRE_PROCESS_MACROS += TIMING_ON
endif

# Check that only one profiler is requested
ifneq ($(and $(findstring LEGACY_TIMER, $(PRE_PROCESS_MACROS)), \
$(findstring VERNIER, $(PRE_PROCESS_MACROS))), )
$(error Multiple profilers specified, limit choice to single profiler.)
endif
```

As a result the following code causes and error when it shouldnt - based upon the existing structure of setting arguments:

```
[[EX1A_BUILD]]
[[[environment]]]
TRANSMUTE_INCLUDE_METHOD = specify_include
PSYCLONE_TRANSMUTE_EXTRAS = '-l all --backend disable-indentation'
VERBOSE = 1
USE_LEGACY_TIMER=false
USE_VERNIER=true
USE_TIMING_WRAPPER=true
[[[directives]]]
-l tmpsize=12GB
```

This can be solved by removing `USE_TIMING_WRAPPER` but I thought worth flagging as it is misleading.

### Relevant log output

```shell

```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in infrastructure/build/lfric.mk around the profiler-selection check and compare its conditions with the USE_* values shown in the EX1A_BUILD configuration. Reproduce the configuration with USE_LEGACY_TIMER=false, USE_VERNIER=true, and USE_TIMING_WRAPPER=true. Done means false-valued options do not trigger the multiple-profiler error while enabled profiler choices still are checked.

Written by the indexing model from the issue text.

Assessment

Tech stack
fortran
Domain
build-system
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.