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

Extract CellSet to ViennaCS Library #82

Merged
merged 6 commits into from
May 30, 2024

Conversation

exilief
Copy link
Contributor

@exilief exilief commented May 27, 2024

This is the implementation for #77.

I removed all the cellSet files except csSurfaceCellSet.hpp (which wasn't moved to ViennaCS yet) and adjusted the includes.
(I accidentally modified psSmartPointer in csSurfaceCellSet.hpp, should I undo those changes?)

What still needs to be changed is all the CMake stuff and Python bindings, and the Readme (and testing if it works).

If the LS and Ray dependencies are replaced with only ViennaCS, it might be a problem, because in tests/CMakeLists.txt and examples/CMakeLists.txt, the VIENNAPS_SYSTEM_VIENNALS and VIENNAPS_SYSTEM_VIENNARAY variables are still used. Should it still do the find_package(ViennaLS) and other calls, even if it's no longer a direct dependency? Or can all 3 libraries be listed as a dependency?
I don't want to mess up the build system, so I'd prefer if you do this part or tell me how to proceed.

@exilief
Copy link
Contributor Author

exilief commented May 27, 2024

I now experimented with updating the build system. In target_link_libraries() in CMakeLists.txt I replaced ViennaLS and ViennaRay with ViennaCS; the other mentions of LS and Ray I left in. This appears to work, but please check if this doesn't break anything else.

If this is okay, then I think only the Python bindings are left to do. In python/pyWrap.cpp the old cellSet location is still used, but I don't know how to deal with this.

@tobre1
Copy link
Member

tobre1 commented May 29, 2024

Please change the target branch for the merge to viennacore.

@exilief exilief changed the base branch from master to viennacore May 30, 2024 06:41
@exilief
Copy link
Contributor Author

exilief commented May 30, 2024

I changed it. Do I need to rebase/merge the conflicts, or can you do that?
Since I already removed the cellSet files, your changes to them will be lost, so should we maybe revert that commit?

@tobre1 tobre1 merged commit 648b443 into ViennaTools:viennacore May 30, 2024
1 of 12 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