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

Add support for passing integer(8) values to mpas_log_write. #1261

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jim-p-w
Copy link
Contributor

@jim-p-w jim-p-w commented Jan 7, 2025

This PR allows callers of mpas_log_write to provide values of type integer(8).

The placeholder specification is $w for integer(8) values, as opposed to $i for integer values.

I ran a test where I interspersed integer and integer(8) values, e.g.

integer, dimension(:), allocatable :: intarray
integer(8), dimension(:), allocatable :: int8array
...
call mpas_log_write('SMIOLf_get_var error $i i81:$w int1:$i i82:$w int2:$i i83:$w', intArgs=[local_ierr, intarray(1), intarray(2)], int8Args=int8array, messageType=MPAS_LOG_ERR)

src/framework/mpas_log.F Outdated Show resolved Hide resolved
src/framework/mpas_log.F Outdated Show resolved Hide resolved
@mgduda
Copy link
Contributor

mgduda commented Jan 7, 2025

@jim-p-w Can you add more detail to the commit message?

@jim-p-w
Copy link
Contributor Author

jim-p-w commented Jan 7, 2025

@jim-p-w Can you add more detail to the commit message?

@mgduda How about:
Allow passing values of type integer(8)/integer(kind=I8KIND) to mpas_log_write.
The string $j is used to identify integer(kind=I8KIND) values.

I'm open to suggestions about what the commit message says.

…log_write.

The string $j is used to identify integer(kind=I8KIND) values.
@abishekg7
Copy link
Collaborator

@mgduda Just for my understanding, would a core infrastructure addition like this require an addition to the unit tests also?

@abishekg7
Copy link
Collaborator

@jim-p-w Probably a basic question, but I'm trying this code snippet out in your branch,

 integer (kind=I8KIND) :: tmp_i8_var = 5123123_I8KIND

 call mpas_log_write('rk_setup i8 kind log $w', int8Args=(/tmp_i8_var/))

and I get this as output. Do I need to set some kind of formatting width here, or am I doing something else wrong perhaps?

 rk_setup i8 kind log **

@jim-p-w
Copy link
Contributor Author

jim-p-w commented Jan 21, 2025

@jim-p-w Probably a basic question, but I'm trying this code snippet out in your branch,

 integer (kind=I8KIND) :: tmp_i8_var = 5123123_I8KIND

 call mpas_log_write('rk_setup i8 kind log $w', int8Args=(/tmp_i8_var/))

and I get this as output. Do I need to set some kind of formatting width here, or am I doing something else wrong perhaps?

 rk_setup i8 kind log **

@abishekg7 I changed the placeholder string to $j instead of $w per Michaels suggestion.

@jim-p-w
Copy link
Contributor Author

jim-p-w commented Jan 21, 2025

@mgduda Just for my understanding, would a core infrastructure addition like this require an addition to the unit tests also?

@abishekg7 @mgduda It wasn't clear to me how to create a unit test for mpas_log_write. When I look at the existing tests in src/core_test I see mpas_log_write is assumed to work, as it is used all over the test code.

@abishekg7
Copy link
Collaborator

@mgduda Just for my understanding, would a core infrastructure addition like this require an addition to the unit tests also?

@abishekg7 @mgduda It wasn't clear to me how to create a unit test for mpas_log_write. When I look at the existing tests in src/core_test I see mpas_log_write is assumed to work, as it is used all over the test code.

Yeah, I also don't see any specific tests for mpas_log_write itself.

Otherwise, the PR looks good. I've tested with NVHPC GPU and CPU versions so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants