Skip to content

MAINT: Replace polyfit/polyval with Polynomial class. #155

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 1, 2022
Merged

MAINT: Replace polyfit/polyval with Polynomial class. #155

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9341046
MAINT: Replace polyfit/polyval with Polynomial class.
rossbar Nov 17, 2022
49430a3
MAINT: Update text to match example.
rossbar Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions content/tutorial-ma.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,25 +264,26 @@ Now, if we want to create a very simple approximation for this data, we should t
dates[~china_total.mask]
```

Finally, we can use the [numpy.polyfit](https://numpy.org/devdocs/reference/generated/numpy.polyfit.html#numpy.polyfit) and [numpy.polyval](https://numpy.org/devdocs/reference/generated/numpy.polyval.html#numpy.polyval) functions to create a cubic polynomial that fits the data as best as possible:
Finally, we can use the
[fitting functionality of the numpy.polynomial](https://numpy.org/doc/stable/reference/generated/numpy.polynomial.polynomial.Polynomial.fit.html)
Copy link
Member

Choose a reason for hiding this comment

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

Could we use intersphinx for links like this? (not in scope in this PR, but this is where I spotted it)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could, but we've chosen not to - the reason being that the MyST extended features are currently tied to sphinx, meaning they only work for the outputs that pass through the sphinx build process. In practice this means that it would render correctly in the website, but not in the Jupyter notebook (i.e. for users running tutorials locally or on binder).

As of now we've decided to limit the tutorials only to syntax that will render correctly in both the browser and the html, but we could certainly revisit this at some point.

PS - this is also one of the major reasons I'm excited about MystJS, though I'm not sure that support for sphinx extensions like intersphinx are on the roadmap (yet).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, gotcha, my ignorance shows how much I dislike working in the browser with notebooks :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are certainly things we could consider that might work within the current constraints. E.g. trying thebe instead of binder for cell-level interactivity rather than notebook-level might work, and would have the advantage that the sphinx-specific functionality (admonitions, links, etc.) could be used more extensively, as readers would be working in the website directly. I don't have any real idea because I've never tried!

Copy link
Member

Choose a reason for hiding this comment

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

No worries about brute forcing a workaround for this.

Adding a link checker CI job would address my main concern about the hard-wired URLs, so I think we should just do that instead (I think we should do it anyway).

package to create a cubic polynomial model that fits the data as best as possible:

```{code-cell}
t = np.arange(len(china_total))
params = np.polyfit(t[~china_total.mask], valid, 3)
cubic_fit = np.polyval(params, t)
model = np.polynomial.Polynomial.fit(t[~china_total.mask], valid, deg=3)
plt.plot(t, china_total)
plt.plot(t, cubic_fit, "--")
plt.plot(t, model(t), "--")
```

This plot is not so readable since the lines seem to be over each other, so let's summarize in a more elaborate plot. We'll plot the real data when
available, and show the cubic fit for unavailable data, using this fit to compute an estimate to the observed number of cases on January 28th 2020, 7 days after the beginning of the records:

```{code-cell}
plt.plot(t, china_total)
plt.plot(t[china_total.mask], cubic_fit[china_total.mask], "--", color="orange")
plt.plot(7, np.polyval(params, 7), "r*")
plt.plot(t[china_total.mask], model(t)[china_total.mask], "--", color="orange")
plt.plot(7, model(7), "r*")
plt.xticks([0, 7, 13], dates[[0, 7, 13]])
plt.yticks([0, np.polyval(params, 7), 10000, 17500])
plt.yticks([0, model(7), 10000, 17500])
plt.legend(["Mainland China", "Cubic estimate", "7 days after start"])
plt.title(
"COVID-19 cumulative cases from Jan 21 to Feb 3 2020 - Mainland China\n"
Expand Down