Skip to content

Comments

Change encoder_convexity weighted summation#2

Merged
shanest merged 8 commits intomainfrom
conv_update
Jul 21, 2025
Merged

Change encoder_convexity weighted summation#2
shanest merged 8 commits intomainfrom
conv_update

Conversation

@Ashvin-Ranjan
Copy link
Collaborator

@Ashvin-Ranjan Ashvin-Ranjan commented Jul 17, 2025

  • Shifted existing function encoder_convexity to skinner_encoder_convexity
  • Changed encoder_convexity to use weighted sum of priors
    • Specifically, uses:
      $$\mathsf{Convexity}(p) = \sum_{y\in Y}p(y)\mathsf{dcon}(p(x|y))$$
  • Added/modified tests to now check for explicit differences between encoder_convexity and skinner_encoder_convexity
  • Adds bug fixes for various issues that have come up, have also added tests for the fixes

- Shifted existing function `encoder_convexity` to `skinner_encoder_convexity`
- Changed `encoder_convexity` to use weighted sum of priors
- Added/modified tests to now check for explicit differences between `encoder_convexity` and `skinner_encoder_convexity`
@Ashvin-Ranjan Ashvin-Ranjan requested a review from shanest July 17, 2025 21:58
Ashvin-Ranjan and others added 7 commits July 17, 2025 14:59
- Fixed issue where struct would not throw error of priors was less than 1
- Added test to ensure this edge case is now covered
- Adds extra error messages to catch zero rows in qwm, which cause errors
- Adds extra testing to ensure the edge case is caught
Copy link
Contributor

@shanest shanest left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks Ash. Only small thing (but not going to make it a blocker) is that in the future I think we'll probably just want to remove skinner_encoder_convexity altogether (possibly with a comment to her repo for good measure)

@shanest shanest merged commit d1d807d into main Jul 21, 2025
2 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.

2 participants