Skip to content

Conversation

@djgagne
Copy link

@djgagne djgagne commented Apr 4, 2025

I am merging updates from Wayne Chuang and myself to the microphysics emulator code and to PUMAS itself. Updates in particular have been made to the quantile transform as well as to the order of variables in the input to the model.

Wayne Chuang and others added 23 commits December 12, 2023 16:45
Adding binary search to quantile transform
commit 8e61d3f
Merge: d2683f2 c9bff7e
Author: Kate Thayer-Calder <[email protected]>
Date:   Mon Mar 10 16:38:26 2025 -0600

    Merge pull request ESCOMP#71 from nusbaume/ccpp_prep

    Remove CAM dependencies and add CCPP layer for PUMAS v1 routines

commit c9bff7e
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Feb 28 10:54:10 2025 -0700

    Add modifications requested during code review.

commit a165913
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Feb 21 16:13:53 2025 -0700

    Move variable initialization to inside GPU-ized loop.

commit 685a20b
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Feb 21 15:51:37 2025 -0700

    Initialize local variables to zero (ESCOMP/PUMAS issue ESCOMP#72).

commit 2b44b58
Merge: 7044b6b a8a10b6
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Feb 7 16:10:25 2025 -0700

    Merge remote-tracking branch 'origin/ccpp_prep' into ccpp_prep

commit 7044b6b
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Feb 7 16:06:45 2025 -0700

    Fix bad dimension in metadata file,
    and change subroutine dummy argument declarations to not specify dimension sizes.

commit a8a10b6
Author: peverwhee <[email protected]>
Date:   Thu Jan 30 14:27:17 2025 -0700

    fix stdnames for a couple of tendency variables

commit fb7bbed
Author: Jesse Nusbaumer <[email protected]>
Date:   Thu Jan 16 14:41:56 2025 -0700

    Fix typo in comment.

commit 7cde106
Author: Jesse Nusbaumer <[email protected]>
Date:   Thu Jan 16 08:47:49 2025 -0700

    Add stub proc_rates DDT metadata file, fix comments, and update Tag Notes.

commit fa085ce
Author: Jesse Nusbaumer <[email protected]>
Date:   Tue Jan 14 16:18:50 2025 -0700

    Fix errors found during a CAM run, and fix micro_pumas_ccpp.meta file using framework script.

commit c5a15a3
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Jan 10 13:51:09 2025 -0700

    Fix syntax errors found during compilation.

commit fd86c7f
Author: Jesse Nusbaumer <[email protected]>
Date:   Thu Jan 9 15:52:31 2025 -0700

    Update dependency metadata listing to include wv_sat_methods.F90.

commit 79cabd3
Author: Jesse Nusbaumer <[email protected]>
Date:   Wed Jan 8 16:06:45 2025 -0700

    Finish CCPP interface for core PUMAS routines.

commit 7a08cd5
Author: Jesse Nusbaumer <[email protected]>
Date:   Mon Dec 23 12:32:29 2024 -0700

    Add remaining input and output arguments to CCPP run (tend) routine.

commit 434778d
Author: Jesse Nusbaumer <[email protected]>
Date:   Fri Dec 6 12:03:09 2024 -0700

    Add input arguments to CCPP run (tend) routine.

commit f820633
Author: Jesse Nusbaumer <[email protected]>
Date:   Tue Nov 19 15:29:46 2024 -0700

    Add CCPP init phase, and perform additional clean-up.

commit 42f2222
Author: Jesse Nusbaumer <[email protected]>
Date:   Wed Nov 13 11:16:57 2024 -0700

    Remove unused 'micro_pumas_data.F90' file.

commit 769f7a3
Author: Jesse Nusbaumer <[email protected]>
Date:   Thu Nov 7 15:46:25 2024 -0700

    First attempt at removing/replacing non-portable CESM share code.
Copy link
Collaborator

@Katetc Katetc left a comment

Choose a reason for hiding this comment

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

Sorry I forgot to hit the green "finish review" button. Here's my issue with the iulog. I don't think this PR should come in until we are at least back to the emulated code working.

proc_rates%qrtend_TAU(1:mgncol,k), &
proc_rates%nctend_TAU(1:mgncol,k), &
proc_rates%nrtend_TAU(1:mgncol,k))
proc_rates%nrtend_TAU(1:mgncol,k), iulog)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, we've removed iulog from the module and only pass it into the init functions of PUMAS as part of abstracting out of the CAM host model (and CCPP-izing). I traced this iulog and it is passed through to one write statement that seems unnecessary. Can we remove it all?

end subroutine quantile_inv_transform

subroutine neural_net_predict(input, neural_net_model, prediction)
subroutine neural_net_predict(input, neural_net_model, prediction, iulog)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to remove this iulog

integer :: input_size
integer :: batch_index_size
integer, allocatable :: batch_indices(:)
integer, intent(in) :: iulog
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this iulog

subroutine tau_emulated_cloud_rain_interactions(qc, nc, qr, nr, rho, lcldm, &
precip_frac, mgncol, q_small, qc_tend, qr_tend, nc_tend, nr_tend)
subroutine tau_emulated_cloud_rain_interactions(qc, nc, qr, nr, pgam, lamc, lamr, n0r, rho, lcldm, &
precip_frac, mgncol, q_small, qc_tend, qr_tend, nc_tend, nr_tend, iulog)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to remove this iulog

real(r8), dimension(batch_size, num_inputs) :: nn_inputs, nn_quantile_inputs
real(r8), dimension(batch_size, num_outputs) :: nn_quantile_outputs, nn_outputs
real(r8), parameter :: dt = 1800.0_r8
integer, intent(in) :: iulog
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove iulog

call quantile_transform(nn_inputs, input_scale_values, nn_quantile_inputs)
call neural_net_predict(nn_quantile_inputs, q_all, nn_quantile_outputs)

call neural_net_predict(nn_quantile_inputs, q_all, nn_quantile_outputs, iulog)
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here's the last iulog to remove, I think

Copy link
Contributor

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

@djgagne Thanks for trying to get this across the finish line! I agree with @Katetc about removing the iulog, and had a few questions/requests of my own, but hopefully none of them will be too difficult to resolve (and if you do have any questions or concerns just let me know). Thanks again!

real(kind = r8) :: mid_val
min_idx = 1
max_idx = size(x)
mid_val = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to initialize mid_val here outside of the while loop? It doesn't look like it is being initialized in binary_search_right below.

If it does need to be initialized, then I would recommend specifying the kind as well, i.e. mid_val = 1_r8.


use module_neural_net, only : Dense, init_neural_net, load_quantile_scale_values
use module_neural_net, only : quantile_transform, quantile_inv_transform, neural_net_predict
use tester, only : write_test_values
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed? It doesn't look like write_test_values is actually being called anywhere.

@@ -0,0 +1,25 @@
module tester
Copy link
Contributor

Choose a reason for hiding this comment

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

If write_test_values is no longer used, then is this Fortran module file needed at all?

Comment on lines +2 to +4
use shr_kind_mod, only: r8=>shr_kind_r8
implicit none
integer, parameter, public :: i8 = selected_int_kind(18)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you decide that this file would be beneficial to keep, then you'll need to make the following modification here in order to maintain PUMAS portability:

Suggested change
use shr_kind_mod, only: r8=>shr_kind_r8
implicit none
integer, parameter, public :: i8 = selected_int_kind(18)
use pumas_kinds, only: r8=>kind_r8
use pumas_kinds, only: i8=>kind_i8
implicit none

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants