-
Notifications
You must be signed in to change notification settings - Fork 35
Example 5: fix parallel .mod race by separate Fortran module dirs per target #460
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
Example 5: fix parallel .mod race by separate Fortran module dirs per target #460
Conversation
|
Thanks @zka26 I'll do a local check of this now, given it isn't run by the CI tests |
jatkinson1000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @zka26 I have run locally and confirm that this is a fix.
I have made 2 suggestions that will allow you to pass the linting which is currently failing.
A small comment in the CMake to note that we need this additional code to place identically named modules in separate directories to avoid conflict might also be a nice touch.
The failing tests are not your fault, and I have opened a patch to resolve this at #461
Once that is merged you should be able to rebase and pass the tests.
Let me know once you make the style changes (feel free to accept the suggestions).
|
@jatkinson1000 Thanks! I have accepted the suggestions and added comments about the additional code. I will rebase once #461 is merged. Also thanks for opening #462, it addresses exactly what I ran into. |
|
Hi @zka26 Thanks for that. |
Set Fortran module output dirs per mod_good and mod_bad and add to include paths
Co-authored-by: Jack Atkinson <[email protected]>
Co-authored-by: Jack Atkinson <[email protected]>
a7c8808 to
0c967a3
Compare
jatkinson1000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zka26
You already pre-empted the review I was writing to fix the executeable names (I guess you checked locally).
Made a couple of suggestions to keep the close brackets on the final line to be consistent with the rest of the file (though I can see an argument for own-line being clearer).
Co-authored-by: Jack Atkinson <[email protected]>
Co-authored-by: Jack Atkinson <[email protected]>
Co-authored-by: Jack Atkinson <[email protected]>
Co-authored-by: Jack Atkinson <[email protected]>
jatkinson1000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about this @zka26 I'm not sure why the linter doesn't like that particular statement, but testing locally these should now pass!
Co-authored-by: Jack Atkinson <[email protected]>
Co-authored-by: Jack Atkinson <[email protected]>
jatkinson1000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, we got there eventually!
Thanks for the submission @zka26 your contribution is really appreciated.
I've marked with hacktoberfest-accepted if you need that and will merge shortly!
|
@jatkinson1000 Thanks! I am glad to see the greens now. Thank you for the opportunity and for the quick suggestions, so it was very fast to apply those changes. |
|
@zka26 out of interest can I ask how you found this code? |
|
@jatkinson1000 I found the repo through Hacktoberfest. |
Summary
Parallel builds of Example 5 create a race: good and bad both define ml_mod.mod into the same folder and in parallel, one target overwrites the other's .mod, which results in error.
Reproduction
Fixes #306
Give each target its own Fortran module output directory and add it to the include path to avoid collisions.
After
Both example5_simplenet_infer_fortran_good and example5_simplenet_infer_fortran_bad build reliably and parallel.
Environment
Note
pt2ts.pysaved the model assaved_simplenet_cpu.pton my run, while the Fortran example expectedsaved_simplenet_model.pt. I solved this by renaming.