UM-PEPL / HallThruster.jl

An open-source fluid Hall thruster code
Other
16 stars 9 forks source link

Wall collisions fix #124

Closed DecBrick closed 4 months ago

DecBrick commented 4 months ago

The major change here is to separate out the wall collisions between the electron energy and momentum calculations. Namely, the electron wall collisions as they contribute to the mobility should be 0 in the plume as there are no walls there while the electron wall collisions as they contribute to the electron energy should still exist in the plume to account for radial losses. By separating out these collision frequencies and storing them separately, we can correctly account for both effects. There is no noticeable performance change between the old and new versions.

Additionally, a warning has been added if the user sets the background neutral pressure, but not the solve background neutrals flag.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.37%. Comparing base (1de9706) to head (91c665e).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124/graphs/tree.svg?width=650&height=150&src=pr&token=ADOlTglgOb&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL)](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL) ```diff @@ Coverage Diff @@ ## main #124 +/- ## ========================================== + Coverage 84.04% 84.37% +0.33% ========================================== Files 36 36 Lines 1968 1978 +10 ========================================== + Hits 1654 1669 +15 + Misses 314 309 -5 ``` | [Files](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL) | Coverage Δ | | |---|---|---| | [src/simulation/simulation.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3NpbXVsYXRpb24vc2ltdWxhdGlvbi5qbA==) | `91.62% <100.00%> (+2.41%)` | :arrow_up: | | [src/simulation/solution.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3NpbXVsYXRpb24vc29sdXRpb24uamw=) | `82.19% <ø> (ø)` | | | [src/simulation/update\_electrons.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3NpbXVsYXRpb24vdXBkYXRlX2VsZWN0cm9ucy5qbA==) | `80.15% <100.00%> (+0.15%)` | :arrow_up: | | [src/wall\_loss\_models/constant\_sheath\_potential.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3dhbGxfbG9zc19tb2RlbHMvY29uc3RhbnRfc2hlYXRoX3BvdGVudGlhbC5qbA==) | `100.00% <100.00%> (ø)` | | | [src/wall\_loss\_models/wall\_losses.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3dhbGxfbG9zc19tb2RlbHMvd2FsbF9sb3NzZXMuamw=) | `75.00% <100.00%> (ø)` | | | [src/wall\_loss\_models/wall\_sheath.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3dhbGxfbG9zc19tb2RlbHMvd2FsbF9zaGVhdGguamw=) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL). Last update [1de9706...91c665e](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/124?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL).
archermarx commented 4 months ago

This is nice! I think i would rename the plume energy scale as something like "radial_loss_frequency" or something similar, however. It's not really a wall collision frequency at this point

DecBrick commented 4 months ago

Well, for the energy loss it still is computed using the wall loss model, even within the channel. Looking at this a little more, I might need to make some more changes as the plume will still compute a SEE coefficient even with these updates. Is there a reference for why the radial losses should use the wall loss model?

archermarx commented 4 months ago

Not really, it's kind of ad hoc. The other model (used in LANDMARK) just assumes a constant sheath potential. Our justification is that the field lines terminate in a wall somewhere. However, it should really be whatever sheath might form at the edge of the plume.

DecBrick commented 4 months ago

That makes sense, although it still sounds like something to come back to later. However, in cross referencing this with the documentation, it does appear that the current implementation of the collision frequency only accounts for singly charged ions, so I'm going to make a commit to update the energy collision frequency name as well as fix the collision frequency.

archermarx commented 4 months ago

Thanks for the update. How much of a difference does this end up making to the overall solution?

DecBrick commented 4 months ago

That should be the last few commits. The wall loss model now correctly accounts for multiply charged species. There is a performance hit of around 5% in runtime using a triply charged model, but that should be a worse-case.