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

Set notebook structure and implemented KTIP algorithm #1040

Closed
wants to merge 0 commits into from

Conversation

joehiggi1758
Copy link
Contributor

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

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.

@seanlaw
Copy link
Contributor

seanlaw commented Oct 30, 2024

@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
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!

@seanlaw
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.

@seanlaw
Copy link
Contributor

seanlaw commented Nov 17, 2024

Moved to #1046

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.

2 participants