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

Remove functions compute_eigvec_from_eigval #171

Closed
wants to merge 2 commits into from

Conversation

eringh
Copy link
Member

@eringh eringh commented Mar 11, 2019

Removes code with reference to the discussion in #154, as the compute_eigvec_from_eigval was quite a bit of code, did not cover all cases, and was a bit hacky.

@jarlebring
Copy link
Member

Nice!

Merging this will have quite a few conflicts once #168 is merged... so if you need to do more modifications to this PR, it might be good to first sync it with the branch errmeasure_rewrite.

@eringh
Copy link
Member Author

eringh commented Mar 12, 2019

I don't think this PR will contain much more. I made it this way to make sure I did not accidentally break something, and to document if we want to go back. We can also wait with merging this until #168 is merged to master (since the latter is more complicated and hence probably nicer to not get merging conflicts). How long do you think that will take?

@jarlebring
Copy link
Member

👍

@eringh
Copy link
Member Author

eringh commented Mar 12, 2019

Conflicts resolved. Unfortunately the diff-tool is not very good at seeing what has happened (so the conflicts were also quite messy to resolve). As soon as the tests are ok I'll merge to master.

@eringh
Copy link
Member Author

eringh commented Mar 12, 2019

The merge was not nice and this seems to messy to clear out. I will create a new branch from master and do the changes again. It was not that difficult, mostly manual work.

@eringh eringh closed this Mar 12, 2019
@eringh eringh deleted the remove_compute_eigvec_from_eigval branch March 12, 2019 12:52
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