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

Fix multiple open of xtbscreen.xyz in local.f90 (xtb/iff test) #1167

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

foxtran
Copy link
Contributor

@foxtran foxtran commented Feb 1, 2025

By some reason, at

if(set%pr_local) call open_file(iscreen,'xtbscreen.xyz','w')
xtbscreen.xyz was opened at first scope level, but the file was closed at second scope level, and therefore, if the following check was failed:
if(maxval(rklmo(5,1:n)).ge.3.)then

the close statement does not work for xtbscreen.xyz and then, at the next iteration, this file was opened at the second time.

Copy link
Contributor

@TyBalduf TyBalduf left a comment

Choose a reason for hiding this comment

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

Do you have an example of an input where this error was occurring and that it's now resolved? Or I guess how did you find this?

The fix looks good/reasonable, I'm just curious if it's a rare case to hit or if it happens fairly regularly and has just gone unnoticed. I know some platforms/compilers are a bit more permissive about having multiple open handles to a file.

@foxtran
Copy link
Contributor Author

foxtran commented Feb 1, 2025

I used xtb/iff test from CMake build system for finding this.

I know some platforms/compilers are a bit more permissive about having multiple open handles to a file.

I modified it.

@marvinfriede marvinfriede self-assigned this Feb 4, 2025
@marvinfriede marvinfriede merged commit 636caec into grimme-lab:main Feb 4, 2025
7 of 13 checks passed
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