Skip to content

Features/ams new interface #92

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

Open
wants to merge 73 commits into
base: develop
Choose a base branch
from
Open

Conversation

koparasy
Copy link
Member

@koparasy koparasy commented Dec 24, 2024

This PR adds the new interface, and allows AMS to read in "Tensors".

Pending Tasks:

  • update examples
  • support rmq for tensors
  • update CI to reflect the changes
    • Update gitlab CI
    • Update github CI
    • Enable RMQ CI
  • Convert examples as external packages. Allowing to link in properly AMS
  • Add tests in which we link as an external project (not the way we do now).
  • Edit the main cmake file
  • Generate models at configure time, instead of build time.
  • Distributed executions should be put back in cause of update-model.
  • Update ams_bench_db.cpp to new testing infrastructure.
  • Remove AMS_CSV from db_types.

After discussion with @rblake-llnl we need the following as well:

  • some convention in dynamically detecting the type/device of the loaded model. I can think of the following approaches:
  1. Add an attribute in the model itself that specifies this, and just set the attribute before freezing (use preserve_attrs) flag on torch freeze to keep this in the jitted model.
  2. Add a JSON option to read that in from the environment.
  • print messages regarding the selected device and model

@koparasy koparasy requested a review from lpottier January 4, 2025 01:32
@koparasy
Copy link
Member Author

koparasy commented Jan 4, 2025

@lpottier I assigned you as a reviewer. This is a complete refactor. I redid many points. In any case reviewing the changes will be hard, I understand. Would you like maybe to setup some meeting and do a code walk through? Let me know.

list(APPEND AMS_APP_LIBRARIES umpire)
list(APPEND AMS_APP_INCLUDES ${UMPIRE_INCLUDE_DIR})

find_package(nlohmann_json REQUIRED)
Copy link
Member Author

Choose a reason for hiding this comment

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

I should always find package in the AMSConfig.cmake.in

@koparasy koparasy force-pushed the features/ams-new-interface branch from df8559d to 0984ccc Compare February 5, 2025 16:06
@koparasy koparasy added this to the Finalize new interface milestone Apr 10, 2025
@koparasy koparasy force-pushed the features/ams-new-interface branch from 4955bdc to e3c093a Compare April 10, 2025 17:43
@koparasy koparasy force-pushed the features/ams-new-interface branch from ab0ca64 to 7e5139f Compare April 16, 2025 18:17
@koparasy koparasy force-pushed the features/ams-new-interface branch from d091d1b to fee3696 Compare April 22, 2025 15:09
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.

1 participant