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

#527: Update JSON generator to work with vt's LBDataHolder #528

Merged

Conversation

tlamonthezie
Copy link
Contributor

@tlamonthezie tlamonthezie commented Sep 11, 2024

Fixes #527
Fixes #531

This PR provides the following changes;

  1. Add packed id support in entities
    1.2 The Object class -> __index renamed as seq_id, + addition of a packed_id field
  2. Add collection_id field
    2.1 Object class: Add the collection_id field to the attributes of the Object class, add objgroup_id field to unused params.
    2.2 JSON Generator: Enable the collection_id field to be set by the JSON generator (JSONDataFilesMaker) (with interactive mode support)
    2.3 set collection_id automatically with some default value of 7 if object is added as migratable to a rank (collection_id required by vt for migratable objects so required for the LBAF Json writer).
    with the help of the new TaskSpecification typed dict defining both time (required) and collection_id (not required)
  3. VTDataWriter: copy object fields to the entity node inside the communication from or to nodes
  4. Adapt unit tests (code, config) to work with these source updates
  5. Update data sets with small entity ids by replacing id by seq_id (considering small ids sequences are in fact seg_id's) regenerate to get data in communiaction endpoints nodes and add some generator config file
  6. Replace shared_block_id by shared_id everywhere

Note: this PR is also needed to adapt to the new LB Data files schema defined by VT PR 2243

IMPORTANT: vt-tv must be updated as well to support seq_id as object identifiers.
Else we have to generate entity id values in LBAF if missing in input data

…ases example compressed data files
@tlamonthezie tlamonthezie force-pushed the 527-update-jons-generator-to-work-with-vt-s-lbdataholder branch from 3d6aae5 to 497ee80 Compare September 16, 2024 12:16
Copy link
Contributor

@ppebay ppebay left a comment

Choose a reason for hiding this comment

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

Thanks for changes @tlamonthezie

@ppebay
Copy link
Contributor

ppebay commented Sep 16, 2024

Requesting now a SNL review @nlslatt @lifflander due to connection with vt

@tlamonthezie tlamonthezie force-pushed the 527-update-jons-generator-to-work-with-vt-s-lbdataholder branch from 8e73870 to c39aecf Compare September 17, 2024 08:51
@tlamonthezie tlamonthezie force-pushed the 527-update-jons-generator-to-work-with-vt-s-lbdataholder branch from c39aecf to ecff71d Compare September 17, 2024 08:54
@tlamonthezie
Copy link
Contributor Author

#531 now included in the PR

Copy link
Contributor

@cwschilly cwschilly left a comment

Choose a reason for hiding this comment

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

The newly generated synthetic-blocks data works with vt 👍

src/lbaf/Model/lbsObject.py Outdated Show resolved Hide resolved
src/lbaf/Model/lbsObject.py Outdated Show resolved Hide resolved
Copy link
Contributor

@cwschilly cwschilly left a comment

Choose a reason for hiding this comment

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

LGTM (and works with vt tests)

Copy link
Contributor

@nlslatt nlslatt Sep 17, 2024

Choose a reason for hiding this comment

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

Why is there a python script to "copy" json files by reading, decompressing, converting to a dict, converting back to a string, compressing, and writing? This seems very inefficient.

Copy link
Contributor Author

@tlamonthezie tlamonthezie Sep 18, 2024

Choose a reason for hiding this comment

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

Hello @nlslatt this script was in fact very helpful to rename keys in some compressed datasets.
The script can be called for example to de-compress a dataset, then you can update the dataset using a text editor, then call the script again to compress.
It was particularly helpful in this issue to rename id to seq_id in some compressed datasets (in particular tests datasets)
For future schema updates, it might be useful to keep this script

@cwschilly cwschilly merged commit 58e6972 into develop Sep 20, 2024
10 checks passed
@cwschilly cwschilly deleted the 527-update-jons-generator-to-work-with-vt-s-lbdataholder branch September 20, 2024 18:02
@cwschilly cwschilly mentioned this pull request Sep 20, 2024
@cwschilly cwschilly restored the 527-update-jons-generator-to-work-with-vt-s-lbdataholder branch September 20, 2024 20:05
@cwschilly cwschilly mentioned this pull request Sep 20, 2024
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.

Use shared_id instead of shared_block_id everywhere Update JSON generator to work with vt's LBDataHolder
5 participants