Skip to content

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

Merged
yunjunz merged 3 commits intoinsarlab:mainfrom
scottstanie:fix-text-file
Apr 15, 2023
Merged

create unique text file for solid.for to write to, fix #50#55
yunjunz merged 3 commits intoinsarlab:mainfrom
scottstanie:fix-text-file

Conversation

@scottstanie
Copy link
Copy Markdown
Collaborator

@scottstanie scottstanie commented Apr 14, 2023

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...

fix fortran compile error

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

use the text file argument
@scottstanie
Copy link
Copy Markdown
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,

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 the 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
Copy Markdown
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
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants