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

Update documentation on pt2ts readme to clarify where the Torchscript… #171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jatkinson1000
Copy link
Member

Clarify where the Torchscript model will be saved.

May close #52

Depends on whether we feel we should output the Torchscript to the local place a user is running pt2ts from (currently what this PR clarifies) or to the place the pt2ts script is located (would require changes to the pt2ts code).

Under principle of least surprise I would go for the former, but this does place it inside the CMake build directory in the examples rather than alongside the model.

I would argue that the examples are something of a contrived case, however, and in reality users will place pt2ts next to their python source and run to obtain the model alongside. They will also likely not be running the Fortran from the same directory as the python model and training in a 'real' application.

Copy link
Contributor

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

Agreed that this is probably the more sensible option, and I think the clarification helps!

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.

Ambiguous description of pt2ts.py
2 participants