-
Notifications
You must be signed in to change notification settings - Fork 6
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
WIP: Learning notebooks #8
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8 +/- ##
==========================================
- Coverage 91.00% 90.75% -0.25%
==========================================
Files 12 12
Lines 1012 963 -49
==========================================
- Hits 921 874 -47
+ Misses 91 89 -2
Continue to review full report at Codecov.
|
Thanks @icweaver this looks like a great start. I think a first order of business will be to implement some less verbose show methods- I'll get on that tonight or tomorrow! Something you might play with is a little interactive slider for radius instead of evaluating distinct curves. |
Always down to show off PlutoUI |
after you rebase on #12 the printing should be a lot cleaner. |
I like it! |
Whoops, didn't mean to close this. I guess trying to rename a branch that is being PRed auto-closes the original? |
99a93be
to
1d9c481
Compare
Ok, should be rebased on top of your current changes now and ready to merge. It's looking like we're going to need some more learning notebooks pretty soon! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking pretty good. For these notebooks we can focus a lot more on practical steps (a la the documentation system) so explaining how to do things and catching common hiccups (like broadcasting) will make future docs even better.
Besides the content, we need to figure out how to actually get this into the documentation. Obviously we can't host a live server, so we need to find a way to incorporate this into the package docs with a small blurb about how to use it as a Pluto notebook.
notebooks/simple_light_curve.jl
Outdated
# ╔═╡ 13e902e0-70bd-11eb-00a2-698092b29b2c | ||
md""" | ||
We will now create a `SimpleOrbit` object, which is a subtype of `AbstractOrbit`. This type accepts the following fields: | ||
""" | ||
|
||
# ╔═╡ db44287e-70bd-11eb-31ea-a1130ca7077f | ||
fieldnames(SimpleOrbit) | ||
|
||
# ╔═╡ f80ed7c2-70bf-11eb-074a-e94f706e5c3b | ||
md""" | ||
Each field is already defined with default values, so we will go ahead and create our `SimpleOrbit` object with a period of 3 days and transit duration of 1 hour, which can be specified seamlessly with the external `Unitful.jl` package: | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of the fieldnames for SimpleOrbit
aren't really user-facing, like half_period
, for example. So I think putting it in documentation might be confusing.
Perhaps another approach could be saying
"First we need to define some orbital parameters" -> "this is facilitated by any of the AbstractOrbits
..." -> "SimpleOrbit
has the most straightforward ... " -> "let's figure out how we can create a SimpleOrbit
by pulling up the docs ?SimpleOrbit
(or via the docs in the bottom right of the Pluto notebook...)" -> "now let's create an orbit with these parameters..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
notebooks/simple_light_curve.jl
Outdated
We can now use this as input into a limb darkening law, which we define next: | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would like to see more explaining what u
is- "these are the coefficients used for the higher order (>0) terms of our limb-darkening law" -> "you'll notice the law we declare has one more term than we gave it; that is because there is always a constant term defined to be -1". some stuff like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
notebooks/simple_light_curve.jl
Outdated
For this example, we are using a Polynomial limb darkening law from [Agol, Luger, Foreman-Mackey (2020)](https://ui.adsabs.harvard.edu/abs/2020AJ....159..123A/abstract). This law is part of the `AbstractLimbDark` supertype, which includes the following laws as well that we will explore in other notebooks: | ||
""" | ||
|
||
# ╔═╡ 832cd21e-70c5-11eb-05b5-cd9a588751fc | ||
subtypes(Transits.AbstractLimbDark) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this example, we are using a Polynomial limb darkening law from [Agol, Luger, Foreman-Mackey (2020)](https://ui.adsabs.harvard.edu/abs/2020AJ....159..123A/abstract). This law is part of the `AbstractLimbDark` supertype, which includes the following laws as well that we will explore in other notebooks: | |
""" | |
# ╔═╡ 832cd21e-70c5-11eb-05b5-cd9a588751fc | |
subtypes(Transits.AbstractLimbDark) | |
For this example, we are using a polynomial limb darkening law from [Agol, Luger, Foreman-Mackey (2020)](https://ui.adsabs.harvard.edu/abs/2020AJ....159..123A/abstract). This law is uses analytically derived integrals which makes it very fast and numerically accurate. There are other limb darkening laws which share a similar `AbstractLimbDark` interface. This interface lets us create composite laws like `IntegratedLimbDark` or `SecondaryLimbDark` with almost no effort. We will explore these laws and more in further notebooks. | |
""" | |
# ╔═╡ 832cd21e-70c5-11eb-05b5-cd9a588751fc | |
subtypes(Transits.AbstractLimbDark) | |
and I would remove the cell with the subtypes
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
notebooks/simple_light_curve.jl
Outdated
md""" | ||
Technically we are passing this to the `compute` function, which is aliased to `ld` for convenience. A row-vector of planet-to-star ratios can be passed as well, and the required light curve computations will be automatically broadcasted: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
md""" | |
Technically we are passing this to the `compute` function, which is aliased to `ld` for convenience. A row-vector of planet-to-star ratios can be passed as well, and the required light curve computations will be automatically broadcasted: | |
md""" | |
All of the limb darkening laws can be called like a function `ld(b, r)` or using the `compute` method- `compute(ld, b, r)`. We provide both for convenience- it's natural and easy to treat the laws as functions but some code is easier to write using the concrete `compute` method. In addition, we don't provide any default vectorized versions of the methods, since Julia does not need to be vectorized for speed (unlike numpy). This lets users apply our laws in a variety of ways (e.g., `map`, `broadcast`, `Threads.@threads`, etc.). Below we use [broadcasting](https://docs.julialang.org/en/v1/manual/arrays/#Broadcasting) with the `@.` macro to create a grid of outputs across the `t` and `rprs` column and row vectors. |
Want to avoid causing undue confusion between compute
and the "functor" style. Also, might be a little more gentle introducing broadcasting- for people coming from python they'll have no idea why everything doesn't just vectorize automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Sounds good, I like the emphasis on pedagogy! Re-working it with that in mind now |
823bfe8
to
e4f3c59
Compare
Ok, changes should be up! (notebook preview) |
notebooks/simple_light_curve.jl
Outdated
md""" | ||
Limb darkening is the natural phenomenon where a star's surface brightness appears to fall off as we look from its center to off-angle towards its limbs. The rate at which it falls off is described by a given limb darkening law. For this example, we are using a polynomial limb darkening law from [Agol, Luger, Foreman-Mackey (2020)](https://ui.adsabs.harvard.edu/abs/2020AJ....159..123A/abstract). This law uses analytically derived integrals, which makes it very fast and numerically accurate. There are other limb darkening laws which share a similar `AbstractLimbDark` interface. This interface lets us create composite laws like `IntegratedLimbDark` or `SecondaryLimbDark` with almost no effort. We will explore these laws and more in further notebooks. | ||
|
||
Pulling up the documentation for our particular law, `PolynomialLimbDark`, we see that it just accepts a single parameter `u`, the vector of limb darkening coefficients. These are defined such that: | ||
|
||
```math | ||
I(\mu) \propto -\sum_{i=0}^N u_i(1 - \mu)^i, \quad u_0 \equiv -1\quad, | ||
``` | ||
|
||
where ``μ`` is a dimensionless parameter that varies from ``1`` to ``0`` as we move from the center to the edge of the star, as seen projected on the sky, ``N`` is the order of the polynomial, ``I`` is the intensity of the star, and ``u_i`` are the components of `u`. | ||
|
||
For this example, we will use a quadratic limb darkening law ``(N = 2)``: | ||
|
||
```math | ||
\begin{align} | ||
I(\mu) &\propto -u_0(1 - \mu)^0 - u_1(1 - μ) - u_2(1 - μ)^2 \\ | ||
&= 1 - u_1(1 - μ) - u_2(1 - μ)^2 \quad. | ||
\end{align} | ||
``` | ||
|
||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all starts to get a little in the weeds and feels out of place. The "textbook" knowledge is more well-suited for docs like the introduction section.
Rather than writing out the math and spelling out the technical explanation of the coefficients, why not just show a plot comparing quadratic to cubic to N=10 order limb darkening curves. (you don't have to explain how you make this plot; just show it). I think this might give more practical knowledge, which is the goal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this change was made to address your comment:
Would like to see more explaining what
u
is- "these are the coefficients used for the higher order (>0) terms of our limb-darkening law" -> "you'll notice the law we declare has one more term than we gave it; that is because there is always a constant term defined to be -1". some stuff like that.
It's not entirely clear to me where you would like to draw the line between practical application and going into the weeds. I think it might be more productive if you just added the specific section you would like to see here when you have the time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the line I am trying to draw is this is practical knowledge, not as much theoretical knowledge. So I think the goal of the tutorial should be "what is a PolynomialLimbDark, how do I make one, what are the parameters, how does it look". So, e.g. you point out there's an extra term in the struct because that might surprise someone when they're actually working with the package, but I don't think we need to take the extra step and show why the derivation causes that, if that makes sense.
So I still think we can cut out the latex for formula and stick to the description that describes it as a law "describing how the surface brightness changes from the center to the limb" and pair that with a little plot comparing orders and I think that's a good line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, the notebook should be updated now
okay I really like where it's at now! How should we go about incorporating it into the docs? I noticed you've set up the installs locally, is there a setup with binder or something which will work? |
Cool =] I can start taking a look into binder later this week. I know there's been a lot of discussion about it over on zulip, and Fons and the rest of the Pluto devs have a really neat implementation on the new Computational Thinking site that we might want to emulate. Saving this convo from slack: |
Something you'll want to consider is the static vs. dynamic output, too. For example, currently the transit curve in the bottom of your notebook is flat showing |
Right, we can do that in the meantime. Saving the notebook with some of the curves showing first should also accomplish this, albeit with the scrubber not reflecting the current state. I thought I already did this in the last commit, is it not showing up for you in the static preview? |
Apparently html previews with interactive sliders are going to be discussed at PlutoCon. I would definitely be interested in seeing if we can apply it to our notebooks here! Adding relevant link here: https://github.com/JuliaPluto/PlutoSliderServer.jl |
Switched branch name to #28 |
Here's a first pass at an intro notebook! Preview here: https://htmlpreview.github.io/?https://raw.githubusercontent.com/icweaver/Transits.jl/learning_notebooks/notebooks/simple_light_curve.jl.html