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

docs: reordered docs for solvers and kernels #759

Merged
merged 14 commits into from
Oct 2, 2024
Merged

Conversation

db091756
Copy link
Member

@db091756 db091756 commented Aug 30, 2024

PR Type

  • Documentation content changes

Description

Reordered docs for solvers and kernels

How Has This Been Tested?

Does this PR introduce a breaking change?

(Write your answer here.)

Screenshots

(Write your answer here.)

Checklist before requesting a review

  • I have made sure that my PR is not a duplicate.
  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have performed a self-review of my code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@db091756 db091756 linked an issue Aug 30, 2024 that may be closed by this pull request
Copy link
Contributor

Performance review

No significant changes to performance.

@tp832944
Copy link
Contributor

@db091756 merge conflict to resolve. I haven't started reviewing yet.

@tp832944 tp832944 self-requested a review August 30, 2024 15:11
@db091756
Copy link
Member Author

@db091756 merge conflict to resolve. I haven't started reviewing yet.

Done this now @tp832944

Copy link
Contributor

Performance review

No significant changes to performance.

coreax/kernels/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

Copy link
Contributor

@tp832944 tp832944 left a comment

Choose a reason for hiding this comment

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

I've requested a few documentation fixes local to what you've been working on. We probably shouldn't propagate them too widely, in order to keep this PR focussed.

coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

Statistically significant changes

  • basic_herding:
    • OLD: compilation 0.3684 units ± 0.04853 units; execution 2.59 units ± 0.02915 units
    • NEW: compilation 0.4239 units ± 0.02871 units; execution 2.532 units ± 0.02353 units
    • Significant increase in compilation time (15.06%, p=0.007331)

Normalisation values for new data:
Compilation: 1 unit = 513.82 ms
Execution: 1 unit = 684.47 ms

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

@db091756
Copy link
Member Author

db091756 commented Sep 4, 2024

Merged with main, fixed those docs problems and a some small missed issues with the jaxtyping PR. Also fixed a Data instance method that cropped up when improving missed jaxtyping.

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

2 similar comments
Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Performance review

No significant changes to performance.

@db091756 db091756 changed the base branch from main to bug/score-matching-slowdown September 4, 2024 14:36
@db091756 db091756 changed the base branch from bug/score-matching-slowdown to main September 4, 2024 14:36
Copy link
Contributor

github-actions bot commented Sep 8, 2024

Performance review

Commit b71676f - Merge df6871b into df8f4fc

No significant changes to performance.

Copy link
Contributor

@tp832944 tp832944 left a comment

Choose a reason for hiding this comment

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

You only need to reply to my duplicate comments once. I've put one on each line to make sure they're not missed.

coreax/data.py Outdated Show resolved Hide resolved
coreax/data.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Outdated Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
coreax/kernels/scalar_valued.py Show resolved Hide resolved
Copy link
Contributor

Performance review

Commit 2ffd69f - Merge e6311e4 into 41b3922

No significant changes to performance.

Copy link
Contributor

Performance review

Commit ed651ed - Merge 24a7fbc into 41b3922

No significant changes to performance.

Copy link
Contributor

@tp832944 tp832944 left a comment

Choose a reason for hiding this comment

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

Just the conversation regarding overrides and annotations remaining.

@tp832944 tp832944 merged commit 82ab5ec into main Oct 2, 2024
23 checks passed
@tp832944 tp832944 deleted the docs/reorder-files branch October 2, 2024 11:57
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.

Reorder/refactor kernels and solvers to match documentation order
2 participants