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

[refactor] better organise namespace #896

Merged
merged 70 commits into from
Oct 26, 2023
Merged

[refactor] better organise namespace #896

merged 70 commits into from
Oct 26, 2023

Conversation

toxa81
Copy link
Collaborator

@toxa81 toxa81 commented Sep 28, 2023

  • TERMINATE() macro is removed; use RTE_THROW instead
  • STOP() macro removed; use RTE_THROW instead
  • WARNING() macro is removed; use RTE_WARNING
  • in many paces assert() macro is replaced by RTE_ASSERT()
  • utils:: namespace is removed
    • math functions are moved to math_tools.hpp
    • time functions are moved to time_tools.hpp
    • output stream functions are moved to ostream_tools.hpp
    • string functions are moved to string_tools.hpp
  • some files are moved to ./core folder in preparation to remove SDDK
  • splindex class is moved out of sddk:: to sirius:: namespace
  • old interface to Plasma eigensolver is removed
  • some unused gpu kernels are removed
  • acc:: namespace is re-organised and moved to ./core

This PR touches a lot of files, but it doesn't change the logic of the code. Only source file reshuffling and namespace organisation. Unused cuda code is removed. What is left is la:: space, but this will be done in separate PR after #900

@toxa81
Copy link
Collaborator Author

toxa81 commented Oct 2, 2023

cscs-ci run default

@toxa81
Copy link
Collaborator Author

toxa81 commented Oct 24, 2023

image

current layout of the namespaces. I'll keep la:: namespace untouched before we merge DLF Rocco's PR.

@toxa81 toxa81 changed the title [refactor] utils:: namespace [refactor] better organise namespace Oct 24, 2023
@toxa81 toxa81 marked this pull request as ready for review October 25, 2023 07:59
@toxa81 toxa81 self-assigned this Oct 25, 2023
Copy link
Collaborator

@simonpintarelli simonpintarelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@mtaillefumier mtaillefumier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can just put ETH Zurich / CSCS instead of the authors because we can always trace the authors back with github and commits.

apps/mini_app/sirius.scf.cpp Outdated Show resolved Hide resolved
@simonpintarelli
Copy link
Collaborator

simonpintarelli commented Oct 25, 2023

I wonder if we can just put ETH Zurich / CSCS instead of the authors because we can always trace the authors back with github and commits.

The git blame is not necessarily the same as the author, due to reformatting or other cosmetic changes. But is a global license file not enough? It would probably safe a few kb to remove the boiler plate header :)

@toxa81
Copy link
Collaborator Author

toxa81 commented Oct 25, 2023

  1. I saw in other codes that each file is equipped with boile plate
  2. I will be happy to switch to ETH Zurich / CSCS ownership or even sirius-org. I just need to find out if I can "just do it".

@mtaillefumier
Copy link
Collaborator

I wonder if we can just put ETH Zurich / CSCS instead of the authors because we can always trace the authors back with github and commits.

The git blame is not necessarily the same as the author, due to reformatting or other cosmetic changes.It does not scale with projects that have far too many devs. We are three so it is ok.

@mtaillefumier
Copy link
Collaborator

we do not have to do it now, I wanted to put the question on the table because we will eventually have to do something like this. I guess we can just leave it as is until SIRIUS really see external developers contribute to it.

@mtaillefumier
Copy link
Collaborator

Licencing each file is also fine.

@toxa81 toxa81 merged commit 7671a0a into develop Oct 26, 2023
3 checks passed
@toxa81 toxa81 deleted the refactor/macros branch October 26, 2023 05:28
This was referenced Oct 27, 2023
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.

3 participants