Skip to content

minor fixes to issues detected on fugaku installation #484

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.

Sign up for GitHub

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

Open
pdziekan wants to merge 5 commits into
base: master
Choose a base branch
from
Open

minor fixes to issues detected on fugaku installation #484

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
19d5833
minor fixes to issues detected on fugaku installation
pdziekan Mar 26, 2026
9f15afe
Merge branch 'master' into fugaku_fixes
pdziekan Apr 8, 2026
65bd755
Merge branch 'master' into fugaku_fixes
pdziekan Apr 15, 2026
642c2bc
fix cond adaptation condition for drw2_max
pdziekan Apr 16, 2026
b6d9d99
Merge branch 'master' into fugaku_fixes
pdziekan Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/libcloudph++/common/ice_nucleation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace libcloudphxx

if (INP_type == INP_t::mineral && A > real_t(1e-20))
{
return (real_t(273.15) + (real_t(8.934) - log(- log(real_t(1.) - rand) / A) ) / real_t(0.517)) * si::kelvin;
return real_t(real_t(273.15) + (real_t(8.934) - log(- log(real_t(1.) - rand) / A) ) / real_t(0.517)) * si::kelvin;
}
Comment on lines 36 to 38
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return statement appears to be missing a closing ) for the outer real_t( cast, which will fail to compile. Even with the parenthesis fixed, casting the unit-bearing expression to real_t would be incorrect here since the function returns quantity<si::temperature, real_t>; the previous form (scalar) * si::kelvin is the appropriate pattern.

Copilot uses AI. Check for mistakes.
else
{
Expand Down
10 changes: 7 additions & 3 deletions src/impl/condensation/perparticle/perparticle_nomixing_adaptive_sstp_cond.ipp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ namespace libcloudphxx
tpl_t tpl
) //noexcept
{
#if !defined(__NVCC__)
using std::abs;
#endif

// copy values into local variables
// variables that are not modified
const real_t sstp_dlt_rv = thrust::get<5>(thrust::get<0>(tpl));
Expand Down Expand Up @@ -160,9 +164,9 @@ namespace libcloudphxx

if(sstp_cond_try > 1) // check for convergence
{
if((cuda::std::abs(drw2_new * 2 - drw2) <= sstp_cond_adapt_drw2_eps * rw2) // drw2 relative to rw2 converged
&& cuda::std::abs(drw2 < sstp_cond_adapt_drw2_max * rw2)) // otherwise for small droplets (near activation?) drw2_new == 2*drw already for 2 substeps, but we ativate too many droplets
// if(cuda::std::abs(drw2_new * 2 - drw2) <= tol * drw2) // drw2 converged
if((abs(drw2_new * 2 - drw2) <= sstp_cond_adapt_drw2_eps * rw2) // drw2 relative to rw2 converged
&& abs(drw2 < sstp_cond_adapt_drw2_max * rw2)) // otherwise for small droplets (near activation?) drw2_new == 2*drw already for 2 substeps, but we ativate too many droplets
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second convergence condition applies abs() to the result of a comparison (drw2 < sstp_cond_adapt_drw2_max * rw2), so it effectively becomes abs(bool) (0/1) rather than bounding the magnitude of drw2. This likely breaks the intended safeguard for small droplets; consider comparing abs(drw2) (or abs(drw2_new) as appropriate) against sstp_cond_adapt_drw2_max * rw2 instead.

Suggested change
&& abs(drw2 < sstp_cond_adapt_drw2_max * rw2)) // otherwise for small droplets (near activation?) drw2_new == 2*drw already for 2 substeps, but we ativate too many droplets
&& (abs(drw2_new) < sstp_cond_adapt_drw2_max * rw2)) // otherwise for small droplets (near activation?) drw2_new == 2*drw already for 2 substeps, but we ativate too many droplets

Copilot uses AI. Check for mistakes.
// if(abs(drw2_new * 2 - drw2) <= tol * drw2) // drw2 converged
{
sstp_cond = sstp_cond_try / 2;
_apply_noncond_perparticle_sstp_delta(-delta_fraction_applied); // revert last addition to get to a state after one step of converged number
Expand Down
Loading