8000 create unique text file for solid.for to write to, fix #50 by scottstanie · Pull Request #55 · insarlab/PySolid · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

create unique text file for solid.for to write to, fix #50 #55

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 15, 2023
Merged

create unique text file for solid.for to write to, fix #50 #55

merged 3 commits into from
Apr 15, 2023

Conversation

scottstanie
Copy link
Collaborator
@scottstanie scottstanie commented Apr 14, 2023 •
edited
Loading

Seems to fix the race condition which happens when multiple processes are running in the same directory that @vbrancat and I also saw, which I see was already opened in #50

I started on directly returning arrays from fortran but gave up on it. Maybe some day...

@scottstanie
create unique text file for solid.for to write to, fix insarlab#50
e1c1cd8 
fix fortran compile error

    Error: IMPLICIT statement at (1) cannot follow data declaration statement at (2)

use the text file argument
@scottstanie
pass txt_file to solid_grid to fix failing CI
f7a7f85
@scottstanie
Copy link
Collaborator Author

now that i've fixed the failing CIs, I'll confirm that it does seem to fix what I was running (which was 6 COMPASS runs at a time)

$ find run_files -type f | xargs --max-procs=6 --max-args=1 bash

...
PYSOLID: read data from text file: /tmp/pysolid_hlm_ffdn.txt

...
PYSOLID: read data from text file: /tmp/pysolid_a4kkhmtd.txt

note that i also got a warning, which can be a separate issue:

/u/aurora-r0/staniewi/repos/PySolid/src/pysolid/grid.py:92: UserWarning: Input line 1 contained no data and will not be counted towards `max_rows=2600`.  This differs from the behaviour in NumPy <=1.22 which counted lines rather than rows.  If desired, the previous behaviour can be achieved by using `itertools.islice`.
Please see the 1.23 release notes for an example on how to do this.  If you wish to ignore this warning, use `warnings.filterwarnings`.  This warning is expected to be removed in the future and is given only once per `loadtxt` call.
  fc = np.loadtxt(txt_file,

yunjunz
yunjunz reviewed Apr 14, 2023
View reviewed changes
Copy link
Member
@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

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

Thank you @scottstanie, great idea to use the tempfile module! The PR looks pretty clean, I only have a few minor comments, could you take a look?

@yunjunz
Copy link
Member
yunjunz commented Apr 14, 2023

note that i also got a warning, which can be a separate issue:

That's an existing warning from numpy, nothing to do with pysolid, you could safely ignore it.

@scottstanie
Copy link
Collaborator Author

Thanks @yunjunz !
So I was going to make these small fixes here... I did some quick fortran searching and got the subroutines to return t 8000 he arrays directly without writing to a file in #56

Would you still want this PR since it's simpler? Or should we just go to the better solution in that other PR?

@yunjunz
Copy link
Member
yunjunz commented Apr 15, 2023

That's great news @scottstanie! I would like both if it's not too much to ask~ The current PR is a simpler change, as you said, I would like to have this in the commit history.

@scottstanie
Copy link
Collaborator Author

that makes sense too! i'll rebase that other PR off of this one once it's merged in.

yunjunz
yunjunz approved these changes Apr 15, 2023
View reviewed changes
Copy link
Member
@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

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

Looks all good to me.

@yunjunz yunjunz merged commit 94bec28 into insarlab:main Apr 15, 2023
@scottstanie scottstanie deleted the fix-text-file branch April 15, 2023 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yunjunz yunjunz yunjunz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scottstanie @yunjunz
0