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

seulex_alg_order_fix #59

Merged
merged 1 commit into from Aug 1, 2023
Merged

seulex_alg_order_fix #59

merged 1 commit into from Aug 1, 2023

Conversation

sjdaines
Copy link
Contributor

@sjdaines sjdaines commented Aug 1, 2023

Guess at a fix for #58 :

Probable typo in

SciMLBase.alg_order(alg::seulex)

method definition

NB: not checked whether alg_order for seulex is correct for this implementation - the implementation in https://dx.doi.org/10.1016/j.combustflame.2016.09.018 uses up to 12th order

Guess at a fix for #58 :

Probable typo in 

    SciMLBase.alg_order(alg::seulex)

method definition

NB: not checked whether alg_order for seulex is correct for this implementation - the implementation in https://dx.doi.org/10.1016/j.combustflame.2016.09.018 uses up to 12th order
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 - seulex_alg_order_fix

Title and Description 👍

The Title and description are clear, concise and helpful The title and description of the pull request are clear and concise. They effectively communicate the purpose of the changes, which is to fix a probable typo in the `alg_order` method definition for the `seulex` algorithm.

Scope of Changes 👍

The changes are narrowly focused The changes in this pull request appear to be narrowly focused on resolving a specific issue. There is no indication that the author is attempting to address multiple issues simultaneously.

Testing 👎

Testing methodology or results are not mentioned The description of the pull request does not mention how the author tested the changes. It is important to include this information to ensure the changes work as expected and do not introduce new issues.

Documentation 👎

Docstrings are missing for the newly added method The newly added method `SciMLBase.alg_order(alg::seulex) = 12` does not have a docstring describing its behavior, arguments, and return values. It is important to include this information for better code readability and maintainability.

Suggested Changes

  1. Please add a docstring to the newly added method SciMLBase.alg_order(alg::seulex) = 12 describing its behavior, arguments, and return values.
  2. Please include information on how you tested the changes and the results of those tests in the description.

Reviewed with AI Maintainer

@ChrisRackauckas ChrisRackauckas merged commit 253a80e into SciML:master Aug 1, 2023
3 of 4 checks passed
@sjdaines sjdaines deleted the patch-1 branch August 1, 2023 18:06
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