Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Set notebook structure and implemented KTIP algorithm #1040

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

Closed
joehiggi1758 wants to merge 0 commits into stumpy-dev:main from joehiggi1758:implement_momp

Conversation

@joehiggi1758
Copy link
Contributor

@joehiggi1758 joehiggi1758 commented Oct 30, 2024

Pull Request Checklist

Below is a simple checklist but please do not hesitate to ask for assistance!

  • Fork, clone, and checkout the newest version of the code
  • Create a new branch
  • Make necessary code changes
  • Install black (i.e., python -m pip install black or conda install -c conda-forge black)
  • Install flake8 (i.e., python -m pip install flake8 or conda install -c conda-forge flake8)
  • Install pytest-cov (i.e., python -m pip install pytest-cov or conda install -c conda-forge pytest-cov)
  • Run black --exclude=".*\.ipynb" --extend-exclude=".venv" --diff ./ in the root stumpy directory
  • Run flake8 --extend-exclude=.venv ./ in the root stumpy directory
  • Run ./setup.sh dev && ./test.sh in the root stumpy directory
  • Reference a Github issue (and create one if one doesn't already exist)

Copy link

Check out this pull request on ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Oct 30, 2024
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.33%. Comparing base (9616686) to head (0f2766c).

Additional details and impacted files
@@ Coverage Diff @@
## main #1040 +/- ##
=======================================
 Coverage 97.33% 97.33% 
=======================================
 Files 89 89 
 Lines 15027 15027 
=======================================
 Hits 14626 14626 
 Misses 401 401 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

seanlaw commented Oct 30, 2024
edited
Loading

@joehiggi1758 I know that this is still a work-in-progress but this looks fantastic! I haven't gone over things in detail but, at a high level, I really like how you're breaking everything down in consumable chunks and also explaining each part very clearly. One thing I would like to ask is if can please cross-reference sections/figures/algorithms within the paper so something like:

The next step is to calculate the lower bound (see Section X.Y and Figures A-C)

This way, we can quickly find where things are within the original source.

Finally, please leave me a comment here when you think it is ready for any type of feedback or if you had any questions. Great job so far!

joehiggi1758 reacted with heart emoji

Copy link
Contributor Author

@seanlaw hope you're having a great one!

I've been fairly busy with work lately, but still wanted to move this forward!

I'm about 85% there, the full MOMP implementation isn't working yet - but open to your feedback up to this point on the structure, breakdown and implementation of each sub function!

Copy link
Contributor

seanlaw commented Nov 15, 2024

Thanks @joehiggi1758 and no problem on any delays. Please allow me some time to review it and provide feedback.

joehiggi1758 reacted with thumbs up emoji

Copy link
Contributor

seanlaw commented Nov 17, 2024

Moved to #1046

joehiggi1758 reacted with hooray emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanlaw seanlaw Awaiting requested review from seanlaw

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /