Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edits to Nlayer cylinder documentation #23

Merged
merged 3 commits into from
Apr 15, 2022
Merged

Edits to Nlayer cylinder documentation #23

merged 3 commits into from
Apr 15, 2022

Conversation

sammzer
Copy link
Collaborator

@sammzer sammzer commented Apr 10, 2022

Made some general edits, added in piece on tuple rho values, added examples with multiple rho, tested all tests (they worked except the multiple rho for flux which is an issue we are aware of).

@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2022

Codecov Report

Merging #23 (ef183b7) into main (46de883) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #23   +/-   ##
=======================================
  Coverage   48.71%   48.71%           
=======================================
  Files          16       16           
  Lines        1170     1170           
=======================================
  Hits          570      570           
  Misses        600      600           
Impacted Files Coverage Δ
...dels/Diffusion Approximation/DAcylinder_layered.jl 99.50% <ø> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@@ -14,7 +14,7 @@
Compute the steady-state fluence in an N-layered cylinder.

# Arguments
- `ρ`: source-detector separation in cylindrical coordinates (distance from middle z-axis of cylinder) (cm⁻¹)
- `ρ`: source-detector separation or tuple of separations in cylindrical coordinates (distance from middle z-axis of cylinder) (cm⁻¹)
Copy link
Owner

@heltonmc heltonmc Apr 11, 2022

Choose a reason for hiding this comment

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

I'm wondering if we should specify what types of the arguments should be explicitly. Like in this case rho can be an AbstractFloat or a Tuple{AbstractFloat} or essentially a tuple of abstract floats. It might make more sense to have these as union of types in the argument so

  • ρ::Union{AbstractFloat, Tuple{AbstractFloat}:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I think that would be a good idea for sure

@heltonmc heltonmc merged commit 386cad0 into main Apr 15, 2022
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.

3 participants