Deprecating mixed precision inner product and kgo changes#501
Deprecating mixed precision inner product and kgo changes#501Matthew Walker (mattatmet) wants to merge 5 commits into
Conversation
github-actions
Bot
added
the
cla-required
Matthew Walker (mattatmet)
added
Linked Core
github-actions
Bot
added
cla-signed
Alistair Pirrie (mo-alistairp)
left a comment
Alistair Pirrie (mo-alistairp)
left a comment
There was a problem hiding this comment.
Happy with these changes from a TCD team/PSyclone perspective - I'm definitely happy to see PSyKAl-lite code be removed, thanks for doing that. I've left one comment in science/gungho/integration-test/cma_test/cma_test_algorithm_mod.x90 but otherwise happy.
| @@ -390,11 +389,11 @@ contains | |||
|
|
|||
| ! Calculate differences | |||
| call invoke( X_minus_Y( dx, x1, x2 ) ) | |||
| call invoke_rdouble_X_innerproduct_X(tmp, dx) | |||
| call invoke( X_innerproduct_X(tmp, dx) ) | |||
There was a problem hiding this comment.
This should be merged into the invoke above
There was a problem hiding this comment.
Thanks for this suggestion Alistair. I've made and tested that change now.
Matthew Walker (mattatmet)
requested review from
a team,
Richard Hill (HengistPodd),
cjohnson-pi,
iboutle,
Matt Shin (matthewrmshin),
Steven Sandbach (ss421) and
thomasmelvin
as code owners
|
This will very likely need a linked jedi PR, I will link it once ready. |
DanStoneMO
added
the
Linked Jedi
DanStoneMO
left a comment
DanStoneMO
left a comment
There was a problem hiding this comment.
lfric-jedi PR is now live at: https://github.com/JCSDA-internal/lfric-jedi/pull/1281
iboutle
left a comment
iboutle
left a comment
There was a problem hiding this comment.
I'll defer to Tom M on whether we're happy with changing the precision of these solver calculations. I'm slightly nervous that they were done in double precision for a reason (and we also do it in Endgame), but also understand that reason may no longer be valid given the different solver setup we have. I'm happy it should be scientifically neutral as long as it's stable. Can you at least please check that the job lfric_atm_nwp_gal9-C896_MG_720t_perfRun0_ex1a_cce_production-32bit works fine with this change
5 participants
PR Summary
Sci/Tech Reviewer: thomasmelvin
Code Reviewer: Oakley Brunt (@oakleybrunt)
This PR is a like-for-like replica of the Trac ticket #376. This involves replacing the mixed precision inner product subroutines with PSyclone built-ins. These replacements have been made mostly in the solver in LFRic core but also in the CMA algorithm integration test in LFRic apps. Due to this fundamental change in core, the KGOs for apps have changed in apps, resulting in this PR.
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - mixed-precision-apps/run1
Suite Information
Task Information
❌ failed tasks - 166
⌛ waiting tasks - 2
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review