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

[features/run] Support for output record logging and parsing #2533

Open
wants to merge 5 commits into
base: features/run
Choose a base branch
from

Conversation

khalatepradnya
Copy link
Collaborator

@khalatepradnya khalatepradnya commented Jan 24, 2025

  • Handle primitive types and arrays of primitive types.
  • Not covered: tuples

Towards: #2526

@khalatepradnya khalatepradnya self-assigned this Jan 24, 2025
github-actions bot pushed a commit that referenced this pull request Jan 30, 2025
@khalatepradnya khalatepradnya force-pushed the record-log-parse branch 2 times, most recently from 2857a05 to 8505897 Compare January 31, 2025 07:57
github-actions bot pushed a commit that referenced this pull request Jan 31, 2025
  -- primitive types
TODO: Handle arrays (vector of results)

* Naive way of parsing integer array log

Signed-off-by: Pradnya Khalate <[email protected]>
github-actions bot pushed a commit that referenced this pull request Feb 2, 2025
@khalatepradnya khalatepradnya marked this pull request as ready for review February 3, 2025 06:43
@khalatepradnya khalatepradnya changed the title [DRAFT] Support for output record logging and parsing Support for output record logging and parsing Feb 3, 2025
@khalatepradnya khalatepradnya changed the title Support for output record logging and parsing [feature] Support for output record logging and parsing Feb 3, 2025
@khalatepradnya khalatepradnya requested a review from sacpis February 3, 2025 20:40
Comment on lines +94 to +103
OutputType extractPrimitiveType(const std::string &label) {
if ('i' == label[0]) {
auto digits = std::stoi(label.substr(1));
if (1 == digits)
return OutputType::BOOL;
return OutputType::INT;
} else if ('f' == label[0])
return OutputType::DOUBLE;
throw std::runtime_error("Unknown datatype in label");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can store this in a map and get the output type if the label is found in the map.

template <typename T>
void addPrimitiveRecord(T value) {
results.emplace_back(
OutputRecord{static_cast<void *>(new T(value)), sizeof(T)});
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of new, we can use unique pointer here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried using std::make_unique to create new value but I couldn't get the value out of it in my tests... :(

@sacpis
Copy link
Collaborator

sacpis commented Feb 3, 2025

I think we can implement object-oriented parser here. We want to handle the different types of objects. Objects being Record, Header, Output. We can have RecordHandler as the parent class with some process method. And then implement that method in the inherited classes.

In the RecordParser, we can have a map of object names and unique pointers of handlers. The parse method will then call the respective process method based on the object type.

We need to give some thought here for nested structure.

@khalatepradnya khalatepradnya changed the title [feature] Support for output record logging and parsing [features/run] Support for output record logging and parsing Feb 4, 2025
…ented for

testing only
* Addressing PR comment

Signed-off-by: Pradnya Khalate <[email protected]>
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