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 unit library due to increasing simulation runtimes. #258

Merged
merged 42 commits into from
Jul 26, 2023

Conversation

Drewniok
Copy link
Collaborator

Description

This PR reverses the use of the unit library. This is necessary because the runtimes of the physical simulation algorithms increase when the unit library is used.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

dependabot bot and others added 30 commits April 2, 2023 15:38
Bumps [libs/Catch2](https://github.com/catchorg/Catch2) from `6783411` to `1f881ab`.
- [Release notes](https://github.com/catchorg/Catch2/releases)
- [Commits](catchorg/Catch2@6783411...1f881ab)

---
updated-dependencies:
- dependency-name: libs/Catch2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [libs/parallel-hashmap](https://github.com/greg7mdp/parallel-hashmap) from `7883cb6` to `d2bed96`.
- [Release notes](https://github.com/greg7mdp/parallel-hashmap/releases)
- [Commits](greg7mdp/parallel-hashmap@7883cb6...d2bed96)

---
updated-dependencies:
- dependency-name: libs/parallel-hashmap
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Drewniok Drewniok added the dependencies Pull requests that update a dependency file label Jul 25, 2023
@Drewniok Drewniok requested a review from marcelwa July 25, 2023 12:43
@Drewniok Drewniok self-assigned this Jul 25, 2023
@Drewniok Drewniok requested review from marcelwa and removed request for marcelwa July 25, 2023 12:44
Copy link
Collaborator

@marcelwa marcelwa left a comment

Choose a reason for hiding this comment

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

Many thanks for the effort. I don't see any immediate problems with it. There are just a couple more places where I would like to see unit hints in the docstrings.

include/fiction/technology/charge_distribution_surface.hpp Outdated Show resolved Hide resolved
include/fiction/technology/charge_distribution_surface.hpp Outdated Show resolved Hide resolved
include/fiction/technology/sidb_defects.hpp Outdated Show resolved Hide resolved
include/fiction/technology/sidb_defects.hpp Outdated Show resolved Hide resolved
include/fiction/utils/math_utils.hpp Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #258 (2f8650a) into main (92bb036) will increase coverage by 0.10%.
The diff coverage is 99.07%.

@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
+ Coverage   94.57%   94.67%   +0.10%     
==========================================
  Files          85       84       -1     
  Lines        7669     7648      -21     
==========================================
- Hits         7253     7241      -12     
+ Misses        416      407       -9     
Files Changed Coverage Δ
...imulation/sidb/calculate_energy_and_state_type.hpp 100.00% <ø> (ø)
...fiction/technology/charge_distribution_surface.hpp 99.61% <97.95%> (+4.06%) ⬆️
...clude/fiction/algorithms/path_finding/distance.hpp 100.00% <100.00%> (ø)
...lgorithms/simulation/sidb/critical_temperature.hpp 89.47% <100.00%> (ø)
...algorithms/simulation/sidb/energy_distribution.hpp 100.00% <100.00%> (ø)
...ion/algorithms/simulation/sidb/is_ground_state.hpp 100.00% <100.00%> (ø)
...tion/algorithms/simulation/sidb/minimum_energy.hpp 100.00% <100.00%> (ø)
.../sidb/occupation_probability_of_excited_states.hpp 96.87% <100.00%> (ø)
...hms/simulation/sidb/sidb_simulation_parameters.hpp 100.00% <100.00%> (+23.52%) ⬆️
...on/algorithms/simulation/sidb/time_to_solution.hpp 96.77% <100.00%> (ø)
... and 6 more

... and 1 file with indirect coverage changes

@Drewniok Drewniok requested a review from marcelwa July 25, 2023 14:32
marcelwa
marcelwa previously approved these changes Jul 25, 2023
@Drewniok Drewniok requested a review from marcelwa July 26, 2023 07:10
@Drewniok Drewniok merged commit 2c1d621 into cda-tum:main Jul 26, 2023
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants