bubbles clean-up: remove R0_type and V0

[hyeoksu-lee]

User description

Description

R0_type is an unnecessary parameter since it only has one valid option: R0_type = 1. As far as I can tell, this has been the case for years. The parameter would only be useful if additional quadrature rules were implemented in the future. For now, I believe its presence may confuse general users. Therefore, I have removed it and documentation has been updated accordingly.

Additionally, I noticed that V0(:) = 1 for model_eqns = 2, but V0(:) = 0 for model_eqns = 4. I suspect that V0(:) = 0 may be a bug.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Scope

• This PR comprises a set of related changes with a common goal

If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.

How Has This Been Tested?

Test suite passes on Carpenter

Checklist

• I have made corresponding changes to the documentation (docs/)
• I ran ./mfc.sh format before committing my code

---

PR Type

Bug fix, Enhancement

---

Description

• Remove unnecessary R0_type parameter from bubble modules

• Fix V0 initialization bug for single bubble case

• Clean up commented code and documentation

• Update examples and test cases

---

Diagram Walkthrough

flowchart LR
  A["R0_type parameter"] -- "remove" --> B["Simplified bubble config"]
  C["V0 = 0 for nb=1"] -- "fix to" --> D["V0 = 1 for nb=1"]
  E["Documentation"] -- "update" --> F["Cleaner bubble docs"]

Loading

File Walkthrough

	Relevant files

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The V0 initialization logic appears inconsistent. For nb == 1, V0 is set to 1.0, but for nb > 1, V0 is also set to 1.0. This suggests the previous V0 = 0.0 for nb == 1 might have been intentional, and changing it could affect bubble dynamics calculations. V0(:) = 1._wp else if (nb > 1) then Logic Issue The condition for calling s_simpson has been simplified from checking both nb > 1 and R0_type == 1 to only checking nb > 1. This removes the R0_type constraint but may call s_simpson in cases where it wasn't intended to be called previously. if (bubbles_euler .and. nb > 1) then call s_simpson(weight, R0) end if

[qodo-merge-pro]

PR Code Suggestions ✨

No code suggestions found for the PR.

[sbryngelson]

probably need @anandrdbz to look at this

[codecov]

Codecov Report

Attention: Patch coverage is 35.29412% with 11 lines in your changes missing coverage. Please review.

Project coverage is 44.04%. Comparing base (c933e72) to head (5eb308a).

Files with missing lines	Patch %	Lines
src/post_process/m_global_parameters.fpp	25.00%	3 Missing ⚠️
src/pre_process/m_global_parameters.fpp	25.00%	3 Missing ⚠️
src/simulation/m_global_parameters.fpp	25.00%	3 Missing ⚠️
src/pre_process/m_assign_variables.fpp	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #963      +/-   ##
==========================================
+ Coverage   44.02%   44.04%   +0.01%     
==========================================
  Files          69       69              
  Lines       19625    19611      -14     
  Branches     2430     2430              
==========================================
- Hits         8640     8637       -3     
+ Misses       9484     9474      -10     
+ Partials     1501     1500       -1     

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

[anandrdbz]

@sbryngelson Yes V0 should be 1 for both cases, I guess we never really use model_eqns = 4 anymore hence this went unnoticed. And yeah R0_type is always 1 since only Simpson's quadrature rule is used

[sbryngelson]

@hyeoksu-lee can you make adjustments based on @anandrdbz's comment? perhaps @anandrdbz you can explain why V0 = 1 so we can add it as a comment to avoid future confusion

[hyeoksu-lee]

@sbryngelson @anandrdbz I just noticed that there is no explanation on patch_icpp()%r0 and patch_icpp()%v0 on the documentation. I think it would be useful if we add some notes on the documentation.

And.. below is my thought on V0 and patch_icpp()%v0.

I don't think V0 and patch_icpp()%v0 are practically useful for the method of classes in polydisperse case. While R0 follows log-normal distribution, assigning V0 = 1 implies that all bubbles with different sizes start with the same initial radial velocity - which does not make much sense to me. It's unclear what kind of physical situation this setup would represent.

In the case of QBMM, we consider the distribution of V, so having either zero or non-zero patch_icpp()%v0 along with V0(:) = 1 makes more sense to me. Still I'm not sure in what situation a non-zero patch_icpp()%v0 would be useful.

In contrast, in monodisperse simulations, having a non-zero V would be much more straightforward and practically meaningful.

[sbryngelson]

@sbryngelson @anandrdbz I just noticed that there is no explanation on patch_icpp()%r0 and patch_icpp()%v0 on the documentation. I think it would be useful if we add some notes on the documentation.

And.. below is my thought on V0 and patch_icpp()%v0.

I don't think V0 and patch_icpp()%v0 are practically useful for the method of classes in polydisperse case. While R0 follows log-normal distribution, assigning V0 = 1 implies that all bubbles with different sizes start with the same initial radial velocity - which does not make much sense to me. It's unclear what kind of physical situation this setup would represent.

In the case of QBMM, we consider the distribution of V, so having either zero or non-zero patch_icpp()%v0 along with V0(:) = 1 makes more sense to me. Still I'm not sure in what situation a non-zero patch_icpp()%v0 would be useful.

In contrast, in monodisperse simulations, having a non-zero V would be much more straightforward and practically meaningful.

Yes right what you say makes sense. If I remember correctly, the case that made more sense is that the velocities of all the bubbles with the same R0 but different R,V (so the QBMM case) start with V0=1. I'm noticing that's not what the original code implements, but feel like there must be some reason for this. @anandrdbz can you comment/guess?

[sbryngelson]

FYI R0_type followed from a code of Keita Ando that had two different strategies for integration and I was messing with that and matching his code. But yeah it's defunct now.

[anandrdbz]

@sbryngelson @hyeoksu-lee

After looking at this carefully, I think V0 is pretty meaningless in the code in the current state. The whole point of using R0 was to get the quadrature nodes at equilibrium radii and then the initial condition is scaled by patch_icpp%r0.

For the bubble velocity there is no such equivalent as they are initialized to a constant value using the case file, so I'm not sure if there is a point to using this extra variable. We don't really get any inherent information about bubble velocities from the log-normal PDF.

[hyeoksu-lee]

@sbryngelson @hyeoksu-lee

After looking at this carefully, I think V0 is pretty meaningless in the code in the current state. The whole point of using R0 was to get the quadrature nodes at equilibrium radii and then the initial condition is scaled by patch_icpp%r0.

For the bubble velocity there is no such equivalent as they are initialized to a constant value using the case file, so I'm not sure if there is a point to using this extra variable. We don't really get any inherent information about bubble velocities from the log-normal PDF.

I agree with @anandrdbz. I can remove V0 from the code if @sbryngelson agrees with this.

[sbryngelson]

@anandrdbz is leading the charge on QBMM/sub-grid bubbles for the few years on our side. so if he says do X, I take his word for it.

[hyeoksu-lee]

@sbryngelson @anandrdbz I removed V0 from the code.