Skip to content

Conversation

@tangwhiap
Copy link
Collaborator

Add the immersion freezing simulation codes, freezing test case, and freezing scenario.

tangwhiap and others added 30 commits August 29, 2023 15:24
Copy link
Collaborator

@jcurtis2 jcurtis2 left a comment

Choose a reason for hiding this comment

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

Just a few more things I've noted while taking a pass through for PyPartMC.


!> compute saturated vapor pressure (units : Pa) with respective to water
!> Formula (10) from [Murphy & Koop, 2004] (https://doi.org/10.1256/qj.04.94)
real(kind=dp) function env_state_saturated_vapor_pressure_water(T)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also wonder why env_state isn't passed to this function given it is an env_state function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because this function only requires the temperature, T is the only necessary input. I thought you had previously suggested that I use T instead of env_state as the input—should I change it back to env_state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you are right - I see where I suggested that. This was probably the simplest suggestion at the time as you had two functions that did the same thing (which isn't a great idea), one taking temperature and the other taking env_state.

I think that made this pretty confusing and in hindsight, we probably should apply a different fix. Not 100% sure what it should be at the moment.

The fact is, that all env_state functions take env_state as an input so this function is an outlier. So I think we need it to take env_state. However we also shouldn't have two functions that do exactly the same thing.

Two paths that I can think of:

  • Make an env_state in the theoretical_freezing.F90 code and then call those functions by passing an env_state that has it the temperature set. Then switch to have env_state as the input rather than temperature.
  • A function that takes env_state that is just a wrapper to call another function that just takes temperature (and this function name shouldn't start with env_state). However, I don't know that we have any cases in the code that does something quite like that. And I'm not quite sure what file this function would actually exist in (it doesn't quite feel like it belongs in util.F90).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your reply! I prefer the second approach since it wouldn’t complicate theoretical_freezing.F90. However, it would still be somewhat of an outlier. We can discuss this further.

src/run_part.F90 Outdated
if (run_part_opt%do_immersion_freezing .and. &
(run_part_opt%immersion_freezing_scheme_type .eq. &
IMMERSION_FREEZING_SCHEME_SINGULAR)) then
call ice_nucleation_singular_initialize(aero_state, aero_data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is now not yet called if using timestep or timeblock variants -> should be moved to the timestep logic

tangwhiap and others added 4 commits September 14, 2025 18:41
Co-authored-by: Jeffrey Curtis <jcurtis2@illinois.edu>
Co-authored-by: Jeffrey Curtis <jcurtis2@illinois.edu>
@slayoo
Copy link
Collaborator

slayoo commented Sep 15, 2025

closing and reopening to trigger codecov analysis in further commits

@slayoo slayoo closed this Sep 15, 2025
@slayoo slayoo reopened this Sep 15, 2025
@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

❌ Patch coverage is 92.99363% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.40%. Comparing base (7e7ac4c) to head (6d962a3).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/aero_particle.F90 66.66% 10 Missing ⚠️
src/ice_nucleation.F90 94.64% 9 Missing ⚠️
src/aero_data.F90 87.50% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #205       +/-   ##
===========================================
+ Coverage        0   77.40%   +77.40%     
===========================================
  Files           0       55       +55     
  Lines           0     9427     +9427     
===========================================
+ Hits            0     7297     +7297     
- Misses          0     2130     +2130     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

add_test(test_tchem ${CMAKE_BINARY_DIR}/test_run/run_test_directory.sh tchem)
endif()
add_test(test_weighting ${CMAKE_BINARY_DIR}/test_run/run_test_directory.sh weighting)
#>>>>>>> 2341ef410d6f49f3169b8461b5fa8c89dbd3c7a2
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems to be a merge-conflict leftover to be removed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be better to make it a part of the web-publishable doxygen html?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC, the lack of these additions was not detected by CI, it would be great to add a mechanism at least parsing all the scenario input on CI

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this is a fairly common occurrence. The urban_plume scenarios are often fixed but the scenarios that often require more libraries (SUNDIALs for condense, CAMP) are often overlooked. This is often as simple as missing a spec file line change/addition. In this case, it would be a lack of adding things in aero_data.dat for most scenarios or a more special case in terms of the CAMP scenario. The CAMP test caught this, it was fixed in the test but not in the scenario til later.

I think that something that simply attempts to run a 0 second simulation will work in terms of reading in all inputs.

aerosol_diameter = aero_particle_dry_diameter(aero_particle, aero_data)
immersed_surface_area = const%pi * aerosol_diameter **2

total_vol = 0d0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
total_vol = 0d0

if (i_spec == aero_data%i_water) cycle
abifm_m = aero_data%abifm_m(i_spec)
abifm_c = aero_data%abifm_c(i_spec)
j_het = 10 ** (abifm_m * (1 - a_w_ice) + abifm_c) * 10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
j_het = 10 ** (abifm_m * (1 - a_w_ice) + abifm_c) * 10000
j_het = 10d0 ** (abifm_m * (1d0 - a_w_ice) + abifm_c) * 10000

Also define what the 10000 is.

Comment on lines +81 to +82
!> Initialization for the sigular scheme, sampling the freezing temperature
!> for each particles.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
!> Initialization for the sigular scheme, sampling the freezing temperature
!> for each particles.
!> Initialization for the singular scheme, sampling the freezing
!> temperature for each particle.

!> for each particles.
subroutine ice_nucleation_singular_initialize(aero_state, aero_data, &
INAS_a, INAS_b)
implicit none
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
implicit none

Comment on lines +161 to +270
subroutine ice_nucleation_immersion_freezing_time_dependent(aero_state, &
aero_data, env_state, del_t, immersion_freezing_scheme_type, &
freezing_rate)

!> Aerosol state.
type(aero_state_t), intent(inout) :: aero_state
!> Aerosol data.
type(aero_data_t), intent(in) :: aero_data
!> Environment state.
type(env_state_t), intent(inout) :: env_state
!> Total time to integrate.
real(kind=dp), intent(in) :: del_t
!> Freezing rate (only used for the constant rate scheme).
real(kind=dp), intent(in) :: freezing_rate
integer, intent(in) :: immersion_freezing_scheme_type

integer :: i_part, i_bin, i_class, n_bins, n_class
real(kind=dp) :: a_w_ice, pis, pvs
real(kind=dp) :: p_freeze

real(kind=dp) :: p_freeze_max, radius_max, diameter_max

integer :: k_th, n_parts_in_bin
real(kind=dp) :: rand
real(kind=dp), allocatable :: H2O_masses(:), total_masses(:), &
H2O_frac(:)
integer :: i_spec_max
real(kind=dp) :: j_het_max
integer :: rand_geo


allocate(total_masses(aero_state_n_part(aero_state)))
allocate(H2O_masses(aero_state_n_part(aero_state)))
allocate(H2O_frac(aero_state_n_part(aero_state)))

call aero_state_sort(aero_state, aero_data)

total_masses = aero_state_masses(aero_state, aero_data)
H2O_masses = aero_state_masses(aero_state, aero_data, include=["H2O"])
H2O_frac = H2O_masses / total_masses
pvs = env_state_saturated_vapor_pressure_wrt_water(env_state%temp)
pis = env_state_saturated_vapor_pressure_wrt_ice(env_state%temp)
a_w_ice = pis / pvs
if (immersion_freezing_scheme_type == IMMERSION_FREEZING_SCHEME_ABIFM) then
call ABIFM_max_spec(aero_data, a_w_ice, i_spec_max, j_het_max)
endif

n_bins = aero_sorted_n_bin(aero_state%aero_sorted)
n_class = aero_sorted_n_class(aero_state%aero_sorted)

loop_bins: do i_bin = 1, n_bins
loop_classes: do i_class = 1, n_class
n_parts_in_bin = integer_varray_n_entry(&
aero_state%aero_sorted%size_class%inverse(i_bin, i_class))
radius_max = aero_state%aero_sorted%bin_grid%edges(i_bin + 1)
diameter_max = radius_max * 2
if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_ABIFM) then
p_freeze_max = ABIFM_Pfrz_max(diameter_max, aero_data, j_het_max, &
del_t)
else if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_CONST) then
p_freeze_max = 1 - exp(freezing_rate * del_t)
endif

k_th = n_parts_in_bin + 1
loop_choosed_particles: do while(.TRUE.)
rand_geo = rand_geometric(p_freeze_max)
k_th = k_th - rand_geo
if (k_th <= 0) then
EXIT loop_choosed_particles
endif
i_part = aero_state%aero_sorted%size_class &
%inverse(i_bin, i_class)%entry(k_th)
if (aero_state%apa%particle(i_part)%frozen) then
cycle
end if
if (H2O_frac(i_part) < const%imf_water_threshold) then
cycle
end if
if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_ABIFM) then
p_freeze = ABIFM_Pfrz_particle( &
aero_state%apa%particle(i_part), aero_data, a_w_ice, del_t)
call warn_assert_msg(301184565, p_freeze <= p_freeze_max,&
"p_freeze > p_freeze_max.")
rand = pmc_random()
if (rand < p_freeze / p_freeze_max) then
aero_state%apa%particle(i_part)%frozen = .TRUE.
aero_state%apa%particle(i_part)%den_ice = &
const%reference_ice_density
aero_state%apa%particle(i_part)%ice_shape_phi = 1d0

endif
else
aero_state%apa%particle(i_part)%frozen = .TRUE.
aero_state%apa%particle(i_part)%den_ice = &
const%reference_ice_density
aero_state%apa%particle(i_part)%ice_shape_phi = 1d0
endif

enddo loop_choosed_particles
enddo loop_classes
enddo loop_bins

deallocate(total_masses)
deallocate(H2O_masses)
deallocate(H2O_frac)

end subroutine ice_nucleation_immersion_freezing_time_dependent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fix all enddo and endif to be end do and end if

del_t)
else if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_CONST) then
p_freeze_max = 1 - exp(freezing_rate * del_t)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
p_freeze_max = 1 - exp(freezing_rate * del_t)
p_freeze_max = 1d0 - exp(freezing_rate * del_t)

H2O_frac(:)
real(kind=dp) :: rand


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +324 to +327
else if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_CONST) then
p_freeze = 1 - exp(freezing_rate * del_t)
end if
Copy link
Collaborator

Choose a reason for hiding this comment

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

p_freeze for IMMERSION_FREEZING_SCHEME_CONST doesn't rely on the particle at all. I think it would be worth considering moving outside the loop.

Comment on lines +221 to +224
else if (immersion_freezing_scheme_type == &
IMMERSION_FREEZING_SCHEME_CONST) then
p_freeze_max = 1 - exp(freezing_rate * del_t)
endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I understand correctly, consider refactoring p_freeze_max outside all these loops for the constant case.

Comment on lines +1370 to +1371
aero_state_frozen_fraction = sum(pack(particle_num_concs, particle_frozen)) &
/ sum(particle_num_concs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It appears that I incorrectly suggested pack here and that we should avoid it when we aren't doing anything extra with the subset of particles. Sum with a mask is much more clear and efficient.

Suggested change
aero_state_frozen_fraction = sum(pack(particle_num_concs, particle_frozen)) &
/ sum(particle_num_concs)
aero_state_frozen_fraction = sum(particle_num_concs, mask=particle_frozen) &
/ sum(particle_num_concs)

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