Skip to content

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Oct 12, 2023

Which issue does this PR close?

N/A

Rationale for this change

While reviewing #7804 (review) I spent time figuring out in more detail what these functions did. I figured I would save myself (and hopefully other future readers) some time in the future

What changes are included in this PR?

Add doc comments and examples for PhysicalExpr::{propagate_constraints, evaluate_bounds}

Are these changes tested?

Docs

Are there any user-facing changes?

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Oct 12, 2023
/// If none of the child intervals change as a result of propagation, may
/// return an empty vector instead of cloning `children`.
///
/// # Example
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@metesynnada and @berkaysynnada -- I would appreciate it if you could double check this example. I think this is the kind of thing propagate_constraints can do, but I am not 100% sure I got the example correct

Copy link
Contributor

Choose a reason for hiding this comment

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

The example you gave is clear and correct, thank you ☺️
For more detail:

  • For plus operation, specifically, we would first do
  • [a_lower, a_upper] <- ([expression_lower, expression_upper] - [b_lower, b_upper]) ∩ [a_lower, a_upper], and then
  • [b_lower, b_upper] <- ([expression_lower, expression_upper] - [a_lower, a_upper]) ∩ [b_lower, b_upper].

@alamb alamb marked this pull request as ready for review October 12, 2023 13:23
@alamb alamb added the documentation Improvements or additions to documentation label Oct 12, 2023
Co-authored-by: Berkay Şahin <124376117+berkaysynnada@users.noreply.github.com>
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Oct 12, 2023
@alamb
Copy link
Contributor Author

alamb commented Oct 16, 2023

@metesynnada or @ozankabak could I trouble you for a review of this PR ? I can't merge it until a committer has marked it as approved

Screenshot 2023-10-16 at 11 19 37 AM

Copy link
Contributor

@ozankabak ozankabak left a comment

Choose a reason for hiding this comment

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

🚀

@alamb
Copy link
Contributor Author

alamb commented Oct 16, 2023

Thank you @ozankabak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
physical-expr Changes to the physical-expr crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants