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

Problem: Eigen3 find script is not updated in master branch / calculation of sigma weights on windows corrupted #23

Closed
wants to merge 11 commits into from

Conversation

rigra
Copy link

@rigra rigra commented Aug 22, 2018

Details:

  1. in develop branch the Eigen3 find script was already updated but not in the master branch
  2. on windows x64 machine, the calculation of the sigmaweights_m does not lead to the sum of 1 which is necessary (W_m_0 + L* W_m_i == 1)
    Solution:
  3. cherry pick the commit of the updated Eigen3 find script
  4. change the calculation of W_m_i to a mathematical equivalent formula with better results on windows x 64

@mherb
Copy link
Owner

mherb commented Aug 22, 2018

First of all, thank you very much for your contribution!

I have a couple of requests here before merging:

  1. Could you make two separate PRs for the eigen cherry-pick and the numerics issue?
  2. I'd prefer to have clean PRs without tons of re-formatting and a dozen commits that actually change a single line, so while re-creating the PRs, could you maybe cleanup the actual changes into a single commit changing only what has actually changed?
  3. (optional) Can you make a unit-test for the numerics issue to avoid future regressions with regard to this?

Thanks!

@rigra
Copy link
Author

rigra commented Sep 3, 2018

Hi,
I will close this PR and set up two new one.

@rigra
Copy link
Author

rigra commented Sep 3, 2018

#26 and #27 substitute this

@rigra rigra closed this Sep 3, 2018
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