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

Fixed argparse for fedsd #273

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Fixed argparse for fedsd #273

merged 5 commits into from
Sep 22, 2023

Conversation

erlingrj
Copy link
Collaborator

The docs are not up-to-date, fedsd doesnt work with fedsd *.lft, you have to do fedsd -r rti.lft -f fed1.lft fed2.lft.

This fixes the argparse setup so that the command line parsing works properly. Previously it turned the list of federate lfts into a double list [[lft1, fft2, ...,]] which crashed the rest of the script

@ChadliaJerad
Copy link
Collaborator

The docs are not up-to-date, fedsd doesn't work with fedsd *.lft, you have to do fedsd -r rti.lft -f fed1.lft fed2.lft.

In the last update, fedsd does not work anymore with fedsd *.lft, but one needs to simply call fedsd instead. The README.md file (under util/tracing/visualization) was updated accordingly. It says:

Once the federation stopped executing, running `fedsd` will operate on all the `.lft` files in the current directory:

and

It is also possible to operate on specific files. In such a case, run `fedsd` with `-r` flag to provide the RTI trace file, and `-f` flag to provide the list of federate trace files. Both arguments are optional.

Actually, I didn't see any value in explicitly saying *.lft, if we need to use all local files.
Does this make sense?

This fixes the argparse setup so that the command line parsing works properly. Previously it turned the list of federate lfts into a double list [[lft1, fft2, ...,]] which crashed the rest of the script

Apologies for the overlook. Thanks for the fix!

Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM.

@erlingrj
Copy link
Collaborator Author

@ChadliaJerad, these are the docs I am referring to: https://www.lf-lang.org/docs/handbook/tracing?target=c#tracing-federated-programs

@ChadliaJerad
Copy link
Collaborator

@ChadliaJerad, these are the docs I am referring to: https://www.lf-lang.org/docs/handbook/tracing?target=c#tracing-federated-programs

I see! This is the handbook PR.

Commit 728e725 fixes some typos, while Commit 9ea185a adjuts the README file contents with this commit.

@erlingrj erlingrj merged commit 64dbf26 into main Sep 22, 2023
28 checks passed
@lhstrh lhstrh added the bugfix label Jan 23, 2024
@lhstrh lhstrh changed the title Fix argparse for fedsd Fixed argparse for fedsd Jan 23, 2024
@erlingrj erlingrj deleted the fedsd-fix branch February 3, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants