DRY m_checker
Closed this issue · 14 comments
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 ifThis 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.
I don't mind doing it. It will take a while though - there are so many checkers!
True. Well you don't have to do it all in one day, up to you.
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.
Okay! I'll give it a try
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
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?
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?
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.
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))
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).
Yes line breaks work in macros and I'll add them. I'll change everything to PROHIBIT then
I'm open to other ideas. Or you can concoct something with @henryleberre and bounce it off me.
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.