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

Script interface Backend Replacement #3794

Merged
merged 147 commits into from
Aug 24, 2020
Merged

Script interface Backend Replacement #3794

merged 147 commits into from
Aug 24, 2020

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Jul 7, 2020

Description of changes

  • Rename all the things
  • Make Variant (shared) own contained objects
  • Introduce separate packing format for sending objects via MPI/serializing
  • Make manager and context concepts explicit
  • Moved script interface unit tests into script interface
  • Added more tests
  • Removed unused Utils::make_function
  • Remvoed unused constructors of ScriptInterface::AutoParameter

@fweik fweik requested a review from KaiSzuttor July 7, 2020 14:33
@fweik fweik changed the title Script interface WIP: Script interface Jul 7, 2020
@fweik fweik changed the title WIP: Script interface WIP: Script interface Backend Replacment Jul 8, 2020
@fweik fweik changed the title WIP: Script interface Backend Replacment WIP: Script interface Backend Replacement Jul 9, 2020
@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #3794 into python will increase coverage by 0%.
The diff coverage is 95%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #3794    +/-   ##
=======================================
  Coverage      89%     89%            
=======================================
  Files         554     560     +6     
  Lines       24166   24023   -143     
=======================================
- Hits        21566   21457   -109     
+ Misses       2600    2566    -34     
Impacted Files Coverage Δ
src/core/io/writer/h5md_specification.cpp 100% <ø> (ø)
src/core/thermostat.hpp 90% <ø> (ø)
src/script_interface/ComFixed.hpp 100% <ø> (ø)
src/script_interface/tests/None_test.cpp 50% <ø> (ø)
src/script_interface/tests/Variant_test.cpp 100% <ø> (ø)
src/script_interface/tests/get_value_test.cpp 100% <ø> (ø)
...tils/include/utils/serialization/unordered_map.hpp 42% <ø> (-58%) ⬇️
src/script_interface/get_value.hpp 87% <66%> (-1%) ⬇️
src/script_interface/shapes/Union.hpp 80% <66%> (ø)
src/script_interface/tests/Exception_test.cpp 66% <66%> (ø)
... and 93 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15504b0...db12614. Read the comment docs.

@fweik
Copy link
Contributor Author

fweik commented Jul 11, 2020

@jngrad I don't understand the sphinx error, can you help me out?

src/script_interface/Context.hpp Show resolved Hide resolved
src/script_interface/Context.hpp Outdated Show resolved Hide resolved
src/script_interface/auto_parameters/AutoParameter.hpp Outdated Show resolved Hide resolved
src/script_interface/auto_parameters/AutoParameter.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@RudolfWeeber RudolfWeeber left a comment

Choose a reason for hiding this comment

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

Didn't read everything, yet.

src/script_interface/ObjectManager.hpp Outdated Show resolved Hide resolved
src/script_interface/PackedVariant.hpp Outdated Show resolved Hide resolved
@fweik
Copy link
Contributor Author

fweik commented Jul 22, 2020

I'd really appreciate it if I could get some feedback on the design.

@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Jul 22, 2020 via email

@fweik
Copy link
Contributor Author

fweik commented Jul 22, 2020

When restoring from serialization, a context is needed to create the objects in, this is why it is on the contex. Now that you mention it, maybe it would be better as a free function, let me try.

The ObjectManager (which is in dire need of a better name) is to deal with the multiple contexts that exist, which is needed e.g. for serialization, because for every object, it needs to be stored in which kind of context it belongs.

@fweik
Copy link
Contributor Author

fweik commented Jul 24, 2020

I've added some docs and move the serialization code to ObjectHandle

src/script_interface/packed_variant.hpp Outdated Show resolved Hide resolved
src/script_interface/Exception.hpp Show resolved Hide resolved
src/script_interface/GlobalContext.cpp Show resolved Hide resolved
src/script_interface/packed_variant.hpp Outdated Show resolved Hide resolved
@jngrad jngrad changed the title WIP: Script interface Backend Replacement Script interface Backend Replacement Aug 20, 2020
Copy link
Member

@jngrad jngrad left a comment

Choose a reason for hiding this comment

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

ok from my side

@fweik
Copy link
Contributor Author

fweik commented Aug 20, 2020

Thank you for adding the docs ;-)

@jngrad jngrad dismissed RudolfWeeber’s stale review August 22, 2020 11:57

Docstrings have been added.

@jngrad jngrad added this to the Espresso 4.2 milestone Aug 22, 2020
@fweik fweik added the automerge Merge with kodiak label Aug 24, 2020
@kodiakhq kodiakhq bot merged commit a2bed7b into espressomd:python Aug 24, 2020
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.

4 participants