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

Add precompilation stage #300

Merged
merged 4 commits into from
Aug 8, 2023
Merged

Add precompilation stage #300

merged 4 commits into from
Aug 8, 2023

Conversation

xtalax
Copy link
Member

@xtalax xtalax commented Aug 8, 2023

Provides a greater than 3x speedup in first run discretization times, at the cost of 6x precompilation time

fixes #293

Copy link

@ai-maintainer ai-maintainer bot left a comment

Choose a reason for hiding this comment

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

AI-Maintainer Review for PR - Add precompilation stage

Title and Description ❌

Title is clear but description is missing
The title of the pull request is clear and indicates the purpose of the changes. However, the description is missing. It would be beneficial to include a description that provides more context about the changes, such as the motivation behind adding the precompilation stage and how it improves the project.

Scope of Changes ✅

Changes are narrowly focused
The changes in this pull request are narrowly focused on adding a precompilation stage. There are no other changes or modifications in the diff, indicating that the author is specifically addressing the addition of the precompilation functionality.

Testing ❌

No information about testing
The description does not provide any information about how the changes were tested. It would be helpful for the author to include information about the testing process, such as the specific test cases used or any additional steps taken to ensure the correctness and functionality of the added precompilation stage.

Code Changes ✅

Code changes are appropriate
The code changes appear to be appropriate for the addition of a precompilation stage. The author has added the `PrecompileTools` package to the project and used it in the `MethodOfLines.jl` file. A new file `precompile.jl` has also been added which contains the precompilation logic.

Suggested Changes

  • Please add a description to the pull request that provides more context about the changes, such as the motivation behind adding the precompilation stage and how it improves the project.
  • Please provide information about how the changes were tested, such as the specific test cases used or any additional steps taken to ensure the correctness and functionality of the added precompilation stage.

Reviewed with AI Maintainer

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #300 (da7fc25) into master (c360747) will increase coverage by 0.74%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   81.56%   82.31%   +0.74%     
==========================================
  Files          36       37       +1     
  Lines        1660     1702      +42     
==========================================
+ Hits         1354     1401      +47     
+ Misses        306      301       -5     
Files Changed Coverage Δ
src/MethodOfLines.jl 100.00% <ø> (ø)
src/precompile.jl 95.23% <95.23%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xtalax xtalax merged commit 135b04d into SciML:master Aug 8, 2023
39 of 42 checks passed
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.

'update_varmap!' not being used
1 participant