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

Eigen over master #721

Closed
wants to merge 8 commits into from

Conversation

Evil-Spirit
Copy link
Collaborator

No description provided.

1. We are making FoldConstants first, so we are copying less amount of data in DeepCopyWithParamsAsPointers.
2. Since we already perform DeepCopyWithParamsAsPointers, PartialWrt already produces params as pointers
@phkahler
Copy link
Member

The CI build is failing because of missing dependency? What do I need? What version? and where should it live?

@Evil-Spirit
Copy link
Collaborator Author

I have no idea how to make CI happy. The problem is what I use Windows, but CI failed for linux. I have Ubuntu only as virtual machine, but have no time to deal with it.

@phkahler
Copy link
Member

phkahler commented Oct 2, 2020

@Evil-Spirit I was able to manually merge the 1st, 4th, and 5th changes listed above. That can be found here:
https://github.com/phkahler/solvespace/tree/solver-patches

I'm tempted to merge that to master if you're OK with that. They are performance related but don't involve Eigen.

@phkahler
Copy link
Member

phkahler commented Oct 2, 2020

I'm tempted to merge that to master if you're OK with that.

@jwesthues or should I not do that? Merging mine would attribute them to me in the log.

@jwesthues
Copy link
Member

I haven't reviewed in detail, but the commits make sense to me conceptually.

@77maxikov
Copy link
Contributor

@phkahler
Copy link
Member

phkahler commented Oct 3, 2020

Maybe switch https://github.com/RLovelett/eigen.git to https://gitlab.com/libeigen/eigen.git may help?

@77maxikov Yeah that link is returning a 404. @Evil-Spirit can you change that URL in CMakeLists.txt to the new eigen over at gitlab as shown in the previous comment and push that to your branch. It should automatically come here and trigger a new CI build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants