Skip to content

Reorganize examples#135

Merged
benegee merged 14 commits intomainfrom
bg/reorganize-examples
Jan 13, 2026
Merged

Reorganize examples#135
benegee merged 14 commits intomainfrom
bg/reorganize-examples

Conversation

@benegee
Copy link
Copy Markdown
Collaborator

@benegee benegee commented Jan 7, 2026

Based on #29, I suggest the following structure for a start.

level 1

distinguish between equations

Current choice:

  • advection
  • euler
  • shallowwater

level 2

distinguish based on geometrical concept

Current choice:

  • cartesian
  • covariant

(for the time being there is only cartesian under euler, which therefore left out)

euler/cartesian level 3

distinguish based on supported species

Current choice:

  • dry_air
  • moist_air
  • precipitation

cartesian_euler level 4

distinguish between test cases

Current choice:

  • buoyancy
  • global_circulation
  • mountain_flow
  • tests

Any feedback highly appreciated, we can still change everything!

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 8, 2026

Codecov Report

❌ Patch coverage is 89.55224% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.10%. Comparing base (bb81c47) to head (5f2add1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/equations/compressible_moist_euler_2d.jl 89.55% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
- Coverage   97.21%   97.10%   -0.12%     
==========================================
  Files          33       33              
  Lines        4491     4558      +67     
==========================================
+ Hits         4366     4426      +60     
- Misses        125      132       +7     

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

@benegee
Copy link
Copy Markdown
Collaborator Author

benegee commented Jan 8, 2026

I also

  • introduced corresponding sub-testsets to get a more detailed test summary output
  • removed some moist Euler tests, which very were similar to others and not needed for coverage
  • moved initial_condition_moist_bubble, which appeared in two elixirs, to equations/ and used this in both elixirs (this causes coverage to decrease because the initial condition has some safeguards, which do not get triggered, and this now affects coverage because I moved it to src/)

@benegee benegee marked this pull request as ready for review January 8, 2026 12:53
@tristanmontoya
Copy link
Copy Markdown
Member

Nice work @benegee! What are your thoughts on just having advection, shallow_water, and euler as the top level (e.g. what physics are being solved)?

For shallow water and maybe advection (although less necessary there because there are fewer tests) there could be a second level of cartesian and covariant. Once we have a covariant Euler solver, there could be a second-level division there too.

I feel like users of TrixiAtmo.jl would probably rather have the choice of physical model be the top level rather than the specifics of the mathematical formulation.

@benegee
Copy link
Copy Markdown
Collaborator Author

benegee commented Jan 12, 2026

Thanks for your feedback @tristanmontoya !

Sounds good to me, and feels somewhat more natural to have the equations at the top level!

@benegee
Copy link
Copy Markdown
Collaborator Author

benegee commented Jan 12, 2026

Here it is!
I also updated the description above.

I am still unsure about "cartesian_manifold". I think it makes sense to separate this from full 3D Cartesian?

@tristanmontoya
Copy link
Copy Markdown
Member

While this is fine with me, what do you think of this modification?

  • Keep the same top level
  • Remove mid-level cartesian under euler until we have other options
  • Make the options under shallow_water and advection into simply covariant and cartesian

@benegee
Copy link
Copy Markdown
Collaborator Author

benegee commented Jan 13, 2026

Thanks again! @tristanmontoya

This make sense because we will not have different Cartesian options under shallow_water and advection, and we can spare one level.

It is also fine with me to not introduce further levels under euler until we have a covariant option.

Copy link
Copy Markdown
Member

@tristanmontoya tristanmontoya left a comment

Choose a reason for hiding this comment

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

Ok, this is good now. The only thing I see is that the example in the README.md should be updated to use the new directory structure as the currently referenced file doesn't exist. I don't care which case we use as the example one, main criteria is that it shouldn't take very long to run.

@benegee benegee merged commit e9a852b into main Jan 13, 2026
7 of 10 checks passed
@benegee benegee deleted the bg/reorganize-examples branch January 13, 2026 15:16
PhilBaa pushed a commit to PhilBaa/TrixiAtmo.jl that referenced this pull request Jan 26, 2026
* reorganize examples

* less AnalysisCallback output

* introduce sub-testsets to get a more detailed summary

* typo

* fmt

* re-use initial_condition_moist_bubble

* fix

* fix

* fix

* removed name from equations file

* change top level

* fix

* fix

* remove cartesian under euler, rename cartesian_manifold
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