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

Make it possible to share maps between both directions #21

Merged
merged 20 commits into from
Oct 5, 2023

Conversation

tmadlener
Copy link
Contributor

@tmadlener tmadlener commented Jul 3, 2023

BEGINRELEASENOTES

  • Generalize the conversion functionality to make it possible to use "generic maps" (i.e. vector<tuple<K, V>> or a proper map<K, V>).
    • This makes most of the conversion a template (i.e. header) library.
    • Keep the current behavior by specifying suitable defaults for all the templates.
  • This is necessary to support the introduction of a shared global map in key4hep/k4MarlinWrapper#147

ENDRELEASENOTES

@tmadlener tmadlener force-pushed the unified-maps branch 3 times, most recently from 628e773 to 0049108 Compare July 6, 2023 11:57
@tmadlener tmadlener force-pushed the unified-maps branch 2 times, most recently from 7be3f9d to 8b7cbd9 Compare September 12, 2023 11:29
@tmadlener tmadlener force-pushed the unified-maps branch 2 times, most recently from e721e6f to ec93a42 Compare September 13, 2023 11:57
@tmadlener
Copy link
Contributor Author

@andresailer @jmcarcell This is in principle ready for review. The diff looks huge, but most of that is d5ae846 and e916827 that shuffle quite a bit of code around.

I keep it as WIP for now until I have also implemented the corresponding changes on the k4MarlinWrapper side, but I don't expect many changes here any longer.

@tmadlener
Copy link
Contributor Author

The changes in the names of the maps in 49dccf2 makes this a breaking change for k4MarlinWrapper

I am not sure if it is worth to try and make this a slightly smoother transition, or whether we just bit the bullet.

@tmadlener tmadlener force-pushed the unified-maps branch 2 times, most recently from 0945324 to d4b4d15 Compare September 27, 2023 17:45
@tmadlener tmadlener changed the title [WIP] Make it possible to share maps between both directions Make it possible to share maps between both directions Sep 28, 2023
Preparation for more general mappnig structure
- Add generic mapInsert method
- Make all functions that take a map templated on the map(s)
  - Make sure type deduction works for vector backed maps and actual
    maps
  - Default template map parameter such that shared library is built
    with that and can be easily linked against

It is now possible to simply swap out the TypeMapT alias to switch to a
vector backed map
- Make conversion functions templated on map
- Default map to what was there before to make it usable as a shared library
@tmadlener tmadlener merged commit a9c3df7 into key4hep:main Oct 5, 2023
2 of 4 checks passed
@tmadlener tmadlener deleted the unified-maps branch October 5, 2023 09:48
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