Skip to content

Add minimal modal run to model dry deposition#211

Open
zdaq12 wants to merge 30 commits intocompdyn:masterfrom
zdaq12:final_modal
Open

Add minimal modal run to model dry deposition#211
zdaq12 wants to merge 30 commits intocompdyn:masterfrom
zdaq12:final_modal

Conversation

@zdaq12
Copy link
Copy Markdown
Contributor

@zdaq12 zdaq12 commented Jul 14, 2025

No description provided.

@slayoo
Copy link
Copy Markdown
Collaborator

slayoo commented Sep 16, 2025

closing and reopening to trigger codecov analysis

@slayoo slayoo closed this Sep 16, 2025
@slayoo slayoo reopened this Sep 16, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 16, 2025

Codecov Report

❌ Patch coverage is 18.91026% with 253 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.50%. Comparing base (e6356e7) to head (3e3b5ec).

Files with missing lines Patch % Lines
src/scenario.F90 34.17% 104 Missing ⚠️
src/output.F90 0.00% 44 Missing ⚠️
src/partmc.F90 0.00% 37 Missing ⚠️
src/run_modal.F90 0.00% 37 Missing ⚠️
src/aero_dist.F90 0.00% 30 Missing ⚠️
src/spec_file.F90 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
- Coverage   77.45%   75.50%   -1.95%     
==========================================
  Files          55       56       +1     
  Lines        9420     9713     +293     
==========================================
+ Hits         7296     7334      +38     
- Misses       2124     2379     +255     

☔ 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.

@zdaq12 zdaq12 marked this pull request as ready for review October 8, 2025 20:03
@jcurtis2 jcurtis2 self-requested a review December 4, 2025 04:47
Copy link
Copy Markdown
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.

Looks pretty good in my first pass.

  • I think we need to add some protections to the modal model. For example, I can attempt to run with emissions/dilution (it won't complain) but they won't do anything. However these are in the spec file (they're zero for the purpose of this scenario but it leads one to think that maybe they could be changed) and they are read in (due to other PartMC subroutines). I think maybe we just add some asserts in partmc_modal to warn/error out on unsupported behavior.
  • Similarly, I could also attempt to turn coagulation on. This isn't actually read anywhere. I guess what we really need is an input_format_modal similar to how we have them for input_format_particle, sectional to say what is supported.
  • We do need to figure out what to do with the scenario. Honestly I'm wondering if we should consider switching this scenario to being a test case (this is what we did with the immersion freezing). This scenario already fits in pretty well with what is found in test/loss. Making it a test would also make codecov happy. Alternatively, I think if it wants to reach scenario levels by making it interesting, we could consider running it for the particle-resolved case and/or sectional case. Just something to consider.

Comment thread src/scenario.F90

#ifdef PMC_USE_QUADPACK
! Do numerical integration when QUADPACK is available
print *, "***USING QUADPACK FOR DRY DEPOSITION***"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this - will print frequently.

Suggested change
print *, "***USING QUADPACK FOR DRY DEPOSITION***"

Comment thread src/scenario.F90
Comment on lines +806 to +809
if (ier > 0) then
call die_msg(837465, "QUADPACK integration failed (error code: " &
// trim(integer_to_string(ier)))
endif
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggest that we switch to assert_msg. Test for ier = zero rather than ier > 0 (unless there is a good reason for testing >0 which would include negatives (if quadpack allows it?)). And replace number with random 9 digit number. Suggestion would look somewhat like the following (admittedly I didn't test it yet)

Suggested change
if (ier > 0) then
call die_msg(837465, "QUADPACK integration failed (error code: " &
// trim(integer_to_string(ier)))
endif
call assert_msg(909106718, ier == 0, &
"QUADPACK integration failed, error code: " &
// trim(integer_to_string(ier)))

Comment thread src/scenario.F90
Comment on lines +477 to +481
if (new_N < 1d0) then
aero_dist%mode(i_mode)%num_conc = 0d0
aero_dist%mode(i_mode)%char_radius = 0d0
cycle
end if
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this some sort of cutoff for small number concentrations? I think a comment would help.

And is it necessary to set the char_radius to 0 or is num_conc = 0 sufficient? I can envision this could cause some issue (maybe currently only in post-processing) if char_radius = 0 is ever used in a lognormal equation.

Comment thread src/scenario.F90
else if (scenario%loss_function_type == SCENARIO_LOSS_FUNCTION_CONSTANT) then
return
else if (scenario%loss_function_type == SCENARIO_LOSS_FUNCTION_DRYDEP) then
! loss
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
! loss

Comment thread src/scenario.F90
Comment on lines +439 to +440
!> Density for each mode.
real(kind=dp), intent(in) :: density
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment says each mode but density is a scalar.

Comment thread src/scenario.F90
!> Dry deposition parameters.
type(drydep_params_t), intent(inout) :: drydep_params

!> \page input_format_chamber Input File Format: Dry Deposition Parameters
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy/pasted \page from elsewhere (chamber.F90). Should be renamed.

Comment thread src/scenario.F90
Comment on lines +461 to +494
do i_mode = 1,aero_dist_n_mode(aero_dist)
aero_mode = aero_dist%mode(i_mode)
N = aero_mode%num_conc

if (N == 0d0) then
cycle
end if

d_pg = aero_mode%char_radius * 2.0d0
ln_sigma_g = aero_mode%log10_std_dev_radius / log10(exp(1.0d0))

! Integrated deposition rate for the 0-th moment (num. conc.)
m_0_rate = -1.0d0 * scenario_integrated_loss_rate_drydep(scenario, aero_mode, &
0.0d0, density, env_state)
new_N = N * exp(m_0_rate * del_t)

if (new_N < 1d0) then
aero_dist%mode(i_mode)%num_conc = 0d0
aero_dist%mode(i_mode)%char_radius = 0d0
cycle
end if

aero_dist%mode(i_mode)%num_conc = new_N

! Integrated deposition rate for the 3-rd moment (proportional to volume conc.)
m_3_rate = -1.0d0 * scenario_integrated_loss_rate_drydep(scenario, aero_mode, 3.0d0, &
density, env_state)
M = N * d_pg**3.0d0 * exp((3.0d0**2.0d0)/2.0d0 * (ln_sigma_g**2.0d0))
new_M = M * exp(m_3_rate * del_t)

! New geometric mean diameter
new_d_pg = (new_M / new_N * exp(-(3.0d0**2.0d0)/2.0d0 * (ln_sigma_g**2.0d0)))**(1.0d0/3.0d0)
aero_dist%mode(i_mode)%char_radius = new_d_pg / 2.0d0
end do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix indentation on the do loop.

@@ -0,0 +1,33 @@
run_type modal # modal run
output_prefix data/modal_dpg_00001000000000000000_sig_2_5_emerson_grass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggest that you change the directory to out (consistent with other scenarios and also specified in the dockerignore)


# The data should have already been generated by ./1_run_dry_dep_modal.sh

../../build/drydep_modal_process
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Complains that it needs an output prefix.

Comment on lines +1 to +2
# dens (kg/m^3) ions in soln (1) molec wght (kg/mole) kappa (1)
SO4 1800 0 96d-3 0.65
Copy link
Copy Markdown
Collaborator

@jcurtis2 jcurtis2 Apr 23, 2026

Choose a reason for hiding this comment

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

Need to tack on the two new freezing parameters. Should be something like this:

Suggested change
# dens (kg/m^3) ions in soln (1) molec wght (kg/mole) kappa (1)
SO4 1800 0 96d-3 0.65
# dens (kg/m^3) ions in soln (1) molec wght (kg/mole) kappa (1) abifm_m (1) abifm_c (1)
SO4 1800 0 96d-3 0.65 0. 0.

Comment thread src/scenario.F90
Comment on lines +1688 to +1689
integer :: total_size, i, N

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like these variables tagged along from elsewhere and aren't needed here (or actually even where you probably got it from). But clean them up here only so this PR stays focused on drydep changes.

Suggested change
integer :: total_size, i, N

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.

3 participants