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

Create M5 notebook #150

Merged
merged 7 commits into from Jun 13, 2023
Merged

Create M5 notebook #150

merged 7 commits into from Jun 13, 2023

Conversation

achoum
Copy link
Collaborator

@achoum achoum commented Jun 8, 2023

Also

  • Remove m5 py example.
  • Fix support for min/max time in plotting
  • Speed-up start_notebook.sh

@achoum achoum marked this pull request as ready for review June 9, 2023 14:29
Copy link
Collaborator

@DonBraulio DonBraulio left a comment

Choose a reason for hiding this comment

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

Cool change! Left minor comments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add this to tutorials/index.md ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A couple typos in the first block:
nodebook, engenering, forecating, specificaly
we shows->we show
will applied->will be applied
over the last , 7, 14 (remove comma)

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Re-run to show interactive plot sales (for some reason they're not shown)

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: make scrollable the cell output where fit is executed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

*Link added to index.md.
*Fix typos.
*Re-run all cells

make scrollable the cell output where fit is executed.
I did not knew about this feature :)

tools/start_notebook.sh Show resolved Hide resolved
input: Guide node. The start and end time boundaries to generate the new
timestamps are defined by the range of timestamps in `input`.
interval: Tick interval.
align: If false, the first tick is generated at the first timestamp
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that the align name here is confusing.
If you think about the begin tick and the sequence of generated ticks, saying that they're "aligned" means that they match at the beginning, but the opposite is true here.
I guess the name comes from aligning ticks to zero? Maybe align_to_zero?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, "align" means that the timestamps will be multiple of "interval".
This is the same as "align" in c++ / asm.
*Adding this to the agenda.

@achoum achoum merged commit d8b2e25 into main Jun 13, 2023
12 checks passed
@achoum achoum deleted the gbm_7 branch June 13, 2023 16:24
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.

None yet

2 participants