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

add option to print GAPC call #167

Merged
merged 8 commits into from
Jan 12, 2023
Merged

add option to print GAPC call #167

merged 8 commits into from
Jan 12, 2023

Conversation

fymue
Copy link
Collaborator

@fymue fymue commented Jan 10, 2023

During our conversation on PR #163, we figured it would be useful to be able to print the GAPC command line call that created the source files for a certain binary directly from that binary, so I added this option into GAPC.

You can now use the -v or --printCall options to instruct the binary to print its GAPC call. As of right now, the program terminates directly after the call is printed, so it is treated like a --help option. But we could of course also just print the call if the option is specified and continue the execution normally afterwards. What are your thoughts?

@fymue fymue requested review from sjanssen2 and kmaibach January 10, 2023 14:47
@sjanssen2
Copy link
Member

how about we always print this information when the user "asks for help" via -h instead of offering another cmd flag, since it is really like a verbose or help message?

@sjanssen2
Copy link
Member

regarding the failing test: I would guess, that your changes break my ugly mechanism in the https://github.com/jlab/fold-grammars/blob/master/Misc/Applications/addRNAoptions.pl script

@kmaibach
Copy link
Member

I prefer the first option (treat it like --help) since I usually want to know which call was used to create the binary before re-executing anything.

@sjanssen2: your suggestion also sounds good, but we'd need to make sure that the message doesn't "disappear" between the other help messages. Maybe put it at the beginning or end?

@fymue
Copy link
Collaborator Author

fymue commented Jan 10, 2023

regarding the failing test: I would guess, that your changes break my ugly mechanism in the https://github.com/jlab/fold-grammars/blob/master/Misc/Applications/addRNAoptions.pl script

The reason the tests failed is that the rnaoptions.hh file gets included instead of the generic_opts.hh file. I assumed that the addRNAoptions.pl script adds the additional arguments from rnaoptions.hh to the generic_opts.hh file (in-place), but it actually does a whole replacement of the file. And since I only added the new checkpointing arguments to the generic_opts.hh file, they obviously don't exist in that case, hence the error.
I guess I have to add them to rnaoptions.hh as well then.

how about we always print this information when the user "asks for help" via -h instead of offering another cmd flag, since it is really like a verbose or help message?

That would work too. Your call.

@sjanssen2
Copy link
Member

how about we always print this information when the user "asks for help" via -h instead of offering another cmd flag, since it is really like a verbose or help message?

That would work too. Your call.

Lets print it in the generic help info then. @kmaibach only fold-grammar binary get the extensive help messages through the addRNAoptions.pl script. But they are typically not meant for the end user and therefore get wrapped by an additional Perl or Python script. Thus, a cluttered help wouldn't hurt.

@sjanssen2
Copy link
Member

The reason the tests failed is that the rnaoptions.hh file gets included instead of the generic_opts.hh file. I assumed that the addRNAoptions.pl script adds the additional arguments from rnaoptions.hh to the generic_opts.hh file (in-place), but it actually does a whole replacement of the file. And since I only added the new checkpointing arguments to the generic_opts.hh file, they obviously don't exist in that case, hence the error.
I guess I have to add them to rnaoptions.hh as well then.

I know, this design isn't perfect. Back then, my fold-grammars repo and Georgs gapc where kind of independent. Today, one could think about moving the RNA options into gapc altogether. However, right now, I would prefer to leave this separation as is. Meaning that it might be easiest to add the same changes for checkpointing to the rnaoptions files in the fold-grammar repo

fymue added a commit that referenced this pull request Jan 10, 2023
@sjanssen2
Copy link
Member

would it be complicated to also add the gapc version, used to compile the sources, like // GAP-C version: // 2023.01.04-checkpointing?

@fymue
Copy link
Collaborator Author

fymue commented Jan 11, 2023

would it be complicated to also add the gapc version, used to compile the sources, like // GAP-C version: // 2023.01.04-checkpointing?

Not at all, I will add that.

Copy link
Member

@sjanssen2 sjanssen2 left a comment

Choose a reason for hiding this comment

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

works like a charm. Thank you!

@fymue fymue merged commit cacf30e into master Jan 12, 2023
fymue added a commit that referenced this pull request Jan 12, 2023
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.

3 participants