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

Fix duty cycle selection #153

Merged
merged 7 commits into from
Jun 6, 2024
Merged

Conversation

Bachibouzouk
Copy link
Collaborator

@Bachibouzouk Bachibouzouk commented Jun 5, 2024

Should be merged prior to merging #152

This PR is designed to be read commit by commit, the explanations are in the commit messages

@Bachibouzouk Bachibouzouk requested a review from FLomb June 5, 2024 20:37
@Bachibouzouk Bachibouzouk mentioned this pull request Jun 5, 2024
4 tasks
or self.cw12[0] <= evaluate < self.cw12[1]
):
if range_within_window(
indexes_low, indexes_high, self.cw11
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@FLomb - in the intermediate commit 510c164 the upper bound is included. In order to compare the two you can locally do git checkout 510c1647237365ada7e3c2f10995301cf46e1e1a and git checkout fix/duty_cycle_selection to come back to the tip of the branch

Copy link
Contributor

@FLomb FLomb left a comment

Choose a reason for hiding this comment

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

Thanks, @Bachibouzouk, for this PR. I agree with your proposed solution, which I have tested with positive outcomes locally.

Previously the mean of the index range was computed and see if it was
falling within a duty cycle. This method compare the boundaries of the
range with the one of each duty cycle to know if there is an
intersection. If there is one, the duty cycle is chosen.

Note: this method does not take into account the case where the indexes
range spans accross more than one duty cycle.
@Bachibouzouk Bachibouzouk force-pushed the fix/duty_cycle_selection branch from 2db0f82 to 2ea6532 Compare June 6, 2024 12:22
@Bachibouzouk Bachibouzouk merged commit fce9fc9 into fix/ramp_convert Jun 6, 2024
1 check passed
@Bachibouzouk Bachibouzouk deleted the fix/duty_cycle_selection branch June 6, 2024 12:28
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