Skip to content

Conversation

@codecov
Copy link

codecov bot commented Dec 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (3bafb8b) to head (678cc30).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #649      +/-   ##
==========================================
+ Coverage   92.93%   98.39%   +5.46%     
==========================================
  Files          40        9      -31     
  Lines        1754      187    -1567     
==========================================
- Hits         1630      184    -1446     
+ Misses        124        3     -121     

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

@slayoo
Copy link
Member

slayoo commented Dec 14, 2025

thanks @yoctoyotta1024 !
how does the problem manifests? Would be great to add a test that fails without this change.

@yoctoyotta1024
Copy link
Author

yoctoyotta1024 commented Dec 15, 2025

thanks @yoctoyotta1024 ! how does the problem manifests? Would be great to add a test that fails without this change.

Hard to say what the test could be, but the issue is that without max_step the dry density profile is slightly different compared to the PySDM Shipway_and_Hill_2012 example (see plot here). This matters for the pressure and density and therefore condensation in the example (because they're so sensitive to the density profile).
dry_density

(note mislabeled units should read kg/m^3)

@slayoo
Copy link
Member

slayoo commented Dec 15, 2025

Thanks! The code generating this plot is a perfect test! Please add it into this PR, and we can work to shape it into a unit test.

@yoctoyotta1024
Copy link
Author

yoctoyotta1024 commented Dec 15, 2025

Thanks! The code generating this plot is a perfect test! Please add it into this PR, and we can work to shape it into a unit test.

Oh the code I ran to make the plot is very dirty and actually an extract from my superdrops-in-action repository (i.e. not PyMPDATA directly). But all I did was add a flag to the Shipway_and_Hill_2012 settings in PyMPDATA which turns on/off the use of a max_step in solve_ivp for the dry density.

Let me just do that more neatly and get back to you.

@yoctoyotta1024
Copy link
Author

I can't reproduce he plot with PyMPDATA because using apprx_drhod_dz=False seems to raise an error? But I've added two more scripts which show basically what I did in my code, hopefully that helps!

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.

2 participants