DRY m_checker

[henryleberre]

From #544 (comment):

Take this example adapted from m_checker (there are literally hundreds of lines like this):

if (acoustic(j)%npulse >= 5 .and. acoustic(j)%dipole) then
   call s_mpi_abort('acoustic('//trim(jStr)//')%dipole is not '// &
                     'supported for support >= 5 '// &
                     '(non-planar supports). Exiting ...')
end if

This can be simplified to:

@:PROHIBIT(acoustic(j)%npulse >= 5 .and. acoustic(j)%dipole, 'for acoustic source '//trim(jStr))

This is what would get printed (I actually ran this):

m_checker.fpp:186: Prohibited condition: acoustic(j)%npulse >= 5 .and. acoustic(j)%dipole. for > acoustic source 1

I think it's much better than our current solution. Less code and less duplication. Of course you can still make this example better but I hope this proves my point.

[sbryngelson]

I feel like @ChrisZYJ had already worked on an issue like this... integrating this would be a nice step up! Let me know if you want to work on it @ChrisZYJ it should be straightforward.

[ChrisZYJ]

I don't mind doing it. It will take a while though - there are so many checkers!

[sbryngelson]

True. Well you don't have to do it all in one day, up to you.

[sbryngelson]

In fact, if you do all of the ones in one of the sub-codes (preprocess, sim., or postprocess), I will merge the PRs one-at-a-time.

[ChrisZYJ]

Okay! I'll give it a try

[ChrisZYJ]

May I know if this is all right or there's a better way?:

@:PROHIBIT(all(fd_order /= (/dflt_int, 1, 2, 4/)), '. Note: fd_order must be 1, 2, or 4')

outputs

Prohibited condition: all(fd_order /= (/dflt_int, 1, 2, 4/)). Note: fd_order must be 1, 2, or 4

[ChrisZYJ]

Also stuff like this are giving me issues:

        if (nGlobal < 2**(min(1, m) + min(1, n) + min(1, p))*num_procs) then
            call s_int_to_str(2**(min(1, m) + min(1, n) + min(1, p))*num_procs, numStr)
            call s_mpi_abort('Total number of cells must be at least '// &
                             '(2^[number of dimensions])*num_procs, which is currently '// &
                             trim(numStr)//'. Exiting ...')
        end if

Should I keep it as it is and not change it to @:PROHIBIT?

[sbryngelson]

Also stuff like this are giving me issues:

        if (nGlobal < 2**(min(1, m) + min(1, n) + min(1, p))*num_procs) then
            call s_int_to_str(2**(min(1, m) + min(1, n) + min(1, p))*num_procs, numStr)
            call s_mpi_abort('Total number of cells must be at least '// &
                             '(2^[number of dimensions])*num_procs, which is currently '// &
                             trim(numStr)//'. Exiting ...')
        end if

Should I keep it as it is and not change it to @:PROHIBIT?

what is the challenge in changing this to prohibit?

[sbryngelson]

w

May I know if this is all right or there's a better way?:

@:PROHIBIT(all(fd_order /= (/dflt_int, 1, 2, 4/)), '. Note: fd_order must be 1, 2, or 4')

outputs

Prohibited condition: all(fd_order /= (/dflt_int, 1, 2, 4/)). Note: fd_order must be 1, 2, or 4

It seems like maybe this should be an ASSERT? Maybe @henryleberre has a more slick idea in mind.

[ChrisZYJ]

Also stuff like this are giving me issues:

        if (nGlobal < 2**(min(1, m) + min(1, n) + min(1, p))*num_procs) then
            call s_int_to_str(2**(min(1, m) + min(1, n) + min(1, p))*num_procs, numStr)
            call s_mpi_abort('Total number of cells must be at least '// &
                             '(2^[number of dimensions])*num_procs, which is currently '// &
                             trim(numStr)//'. Exiting ...')
        end if

Should I keep it as it is and not change it to @:PROHIBIT?

what is the challenge in changing this to prohibit?

Maybe the first part of the "Prohibited condition: ..." message isn't as helpful as the custom message? But yeah it still works (just a bit long):

Prohibited condition: nGlobal < 2**(min(1, m) + min(1, n) + min(1, p))*num_procs. Total number of cells must be at least (2^[number of dimensions])*num_procs, which is currently 64

from the code:

        call s_int_to_str(2**(min(1, m) + min(1, n) + min(1, p))*num_procs, numStr)
        @:PROHIBIT(nGlobal < 2**(min(1, m) + min(1, n) + min(1, p))*num_procs, 'Total number of cells must be at least (2^[number of dimensions])*num_procs, which is currently '//trim(numStr))

[sbryngelson]

It seems like the code you wrote is nicer than what we had before, though you might want to introduce a line break? (assuming this is allowed in macros).

[ChrisZYJ]

Yes line breaks work in macros and I'll add them. I'll change everything to PROHIBIT then

[sbryngelson]

I'm open to other ideas. Or you can concoct something with @henryleberre and bounce it off me.

[ChrisZYJ]

Yeah I'm sure Henry will have better ideas, and I'm happy to improve the code further. Right now I'm submitting what I have as a PR draft and maybe you could check out the result and see if you like it.