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

132 low rank approx update #137

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

zorian15
Copy link
Collaborator

@zorian15 zorian15 commented Oct 9, 2021

Description

@wsdewitt raised the concern of our low-rank approximation code not playing well with our gauge-fixing procedures (spoiler: he was right). I've gone ahead and written some tests for this (that fail as expected). I figured I'd spend some time thinking about this and if there's any workaround -- though I'm in the same boat as Will right now in thinking these two things may not be able to play together. Will leave this up though until we can discuss it.

I've also revamped how this truncated-SVD/low-rank approximation is implemented, moving things from analysis.py over to model.py as suggested.

Closes #132

Tests

Updated test for low-rank approximations on FullyConnected and Escape models.
A test to see if gauge-fixing and truncated-SVD play nicely together -- expected to fail, does fail.

Checklist:

  • The code uses informative and accurate variable and function names
  • The functionality is factored out into functions and methods with logical interfaces
  • Comments are up to date, document intent, and there are no commented-out code blocks
  • Commenting and/or documentation is sufficient for others to be able to understand intent and implementation
  • TODOs have been eliminated from the code
  • The corresponding issue number (e.g. #278) has been searched for in the code to find relevant notes
  • Documentation has been redeployed

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.

does low rank approximation violate gauge conditions?
1 participant