UM-PEPL / HallThruster.jl

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

More refactoring #125

Closed archermarx closed 4 months ago

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 91.66667% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 83.59%. Comparing base (1de9706) to head (71ca751).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125/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/125?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 #125 +/- ## ========================================== - Coverage 84.04% 83.59% -0.46% ========================================== Files 36 35 -1 Lines 1968 1932 -36 ========================================== - Hits 1654 1615 -39 - Misses 314 317 +3 ``` | [Files](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL) | Coverage Δ | | |---|---|---| | [src/HallThruster.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL0hhbGxUaHJ1c3Rlci5qbA==) | `100.00% <ø> (ø)` | | | [src/collisions/collision\_frequencies.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL2NvbGxpc2lvbnMvY29sbGlzaW9uX2ZyZXF1ZW5jaWVzLmps) | `100.00% <ø> (ø)` | | | [src/physics/fluid.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3BoeXNpY3MvZmx1aWQuamw=) | `100.00% <100.00%> (ø)` | | | [src/physics/thermodynamics.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3BoeXNpY3MvdGhlcm1vZHluYW1pY3Muamw=) | `89.47% <100.00%> (-10.53%)` | :arrow_down: | | [src/simulation/configuration.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3NpbXVsYXRpb24vY29uZmlndXJhdGlvbi5qbA==) | `80.00% <100.00%> (+1.31%)` | :arrow_up: | | [src/simulation/simulation.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3NpbXVsYXRpb24vc2ltdWxhdGlvbi5qbA==) | `89.20% <100.00%> (ø)` | | | [src/utilities/utility\_functions.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3V0aWxpdGllcy91dGlsaXR5X2Z1bmN0aW9ucy5qbA==) | `92.13% <100.00%> (-0.18%)` | :arrow_down: | | [src/numerics/flux.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL251bWVyaWNzL2ZsdXguamw=) | `88.75% <96.42%> (-1.14%)` | :arrow_down: | | [src/physics/thermal\_conductivity.jl](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL#diff-c3JjL3BoeXNpY3MvdGhlcm1hbF9jb25kdWN0aXZpdHkuamw=) | `64.00% <57.14%> (+0.36%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?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/125?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=UM-PEPL). Last update [1de9706...71ca751](https://app.codecov.io/gh/UM-PEPL/HallThruster.jl/pull/125?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

Fair, I'm basically treating >5 is infinite here because then I can use a StepRangeLen for the lookup and get a decent performance improvement. Also, the chance anyone ever needs a +5 or greater charge state is pretty minimal, and the Braginskii closure is already wrong, so I think overall it's a reasonable compromise

On Thu, Feb 29, 2024, 4:57 PM DecBrick @.***> wrote:

@.**** commented on this pull request.

In src/physics/thermal_conductivity.jl https://github.com/UM-PEPL/HallThruster.jl/pull/125#discussion_r1508239899 :

""" -const LOOKUP_ZS = [1.0, 2.0, 3.0, 4.0, Inf] +const LOOKUP_ZS = range(1, 5, length = 5)

I'm not sure the end of this should be 5 as Table 1 of Braginskii has 1,2,3,4,Inf

— Reply to this email directly, view it on GitHub https://github.com/UM-PEPL/HallThruster.jl/pull/125#pullrequestreview-1909945511, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOP3QGV34FQXGMWOK7B2NUDYV6R47AVCNFSM6AAAAABEAV6HVKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBZHE2DKNJRGE . You are receiving this because you authored the thread.Message ID: @.***>