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

Promote vs to a Union{Bool, ...} whenever a Boolean value is present #2237

Closed
wants to merge 1 commit into from

Conversation

ven-k
Copy link
Member

@ven-k ven-k commented Aug 22, 2023

  • With MTKModel macro all input vars are parameters. This introduces, boolean values to vs.

Ref: SciML/ModelingToolkitStandardLibrary.jl#213

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 - Promote vs to a Union{Bool, ...} whenever a Boolean value is present

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 promote vs to a Union{Bool, ...} whenever a Boolean value is present. The reference to the issue provides additional context.

Scope of Changes 👍

The changes are narrowly focused

The changes in this pull request are narrowly focused on promoting vs to a Union{Bool, ...} whenever a Boolean value is present. There are no indications that the author is trying to resolve multiple issues simultaneously.

Testing 👎

Testing methodology is not described

The description of the pull request does not mention how the author tested the changes. It's important to include this information to ensure the changes work as expected and do not introduce new issues.

Code Changes 👍

The code changes are appropriate and well-implemented

The code changes in the pull request are appropriate and seem to be well-implemented. The addition of the has_bool flag and the subsequent checks and promotions appear to be correct.

Recommendations

  • Please provide information on how the changes were tested. This could be unit tests, integration tests, or manual testing steps.
  • Consider adding comments in the code to explain the logic behind the changes, especially the new checks and promotions related to Boolean values. This will make the code easier to understand for other developers.

Reviewed with AI Maintainer

@YingboMa
Copy link
Member

The root issue is fixed by #2231 .

@YingboMa YingboMa closed this Sep 20, 2023
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