PrincetonUniversity / SPEC

The Stepped-Pressure Equilibrium Code, an advanced MRxMHD equilibrium solver.
https://princetonuniversity.github.io/SPEC/
GNU General Public License v3.0
25 stars 5 forks source link

Ambiguous error message "error opening derivative matrix file ;" for Lfindzero=2 #206

Open missing-user opened 1 day ago

missing-user commented 1 day ago

The error is triggered when the input file defines Lfindzero=2 and xspec isn't executed from the same working directory. This is especially annoying, when running SPEC from simsopt or using the Python wrapper, where we have less control about what the current working directory is. This does not happen with other Lfindzero options, because they don't need file IO.

Minimal reproducible example

Save https://github.com/PrincetonUniversity/SPEC/files/6552394/Input_0.txt as Input_0.sp in the directory of your choice, in my case Downloads, then run xspec ~/Downloads/Input_0.sp

newton : 0.63 : 0 0 ; |f|= 3.65065E-03 ; time= 0.45s ; log|BB|e= -3.19 newton : : ; ; log|II|o= -4.13 newton : fatal : myid= 0 ; ios.ne.0 ; error opening derivative matrix file ;

Running cd Downloads && xspec Input_0.sp instead works as expected.

Question

Is this expected behavior, aka should SPEC only be usable with files in the current working directory? If so I would suggest clearly indicating that in the error message. Otherwise file path handling for the derivative matrix file should be fixed accordingly.

smiet commented 1 day ago

It is indeed the expectation that the input file is in the directory where the code is executed.

The derivative file is 'hidden', i.e. it is just the filename with a '.' prepended and a '.DF' appended.

I see two ways to resolve

I see most merit in the second, as the simsopt writes many files and we don't want a run to create these in an unexpected location, or experience clobbering issues when simultaneous runs refer to the same file.

That said, SPEC should handle paths more intelligently (currently it throws a MPI-abort hissyfit if the filename does not exist)

missing-user commented 1 day ago

Excellent, I see it the same way (spec should handle the files more intelligently as this fixes more issues than just the simsopt specific usecase), and already started working on the fix.

smiet commented 1 day ago

Sorry, i misspoke a bit, yes the handling in SPEC should be improved, but I still think it is a good idea to require at least that all created files are located where the code is executed.

Spec may refer to a file in a different location, but it should not write there, only in the current folder.

Simsopt could be adapted to copy the file to the pwd if it is in a different path, file and path handling is much easier in python.

missing-user commented 1 day ago

Okay, I think both are reasonable default behaviors, but there is some more subtlety to it: Since filehandling currently just trims away and replaces the extensions, the full path to the input file is still included in ext. Therefore SPEC runs which don't require hidden intermediary files will write their output to ext.sp.h5, aka next to the input file not the cwd.

  1. Saying that all output files should be written to the cwd is therefore a breaking change for users that rely on output files being written to the same directory as the input files.
  2. Only enforcing the behavior for hidden files, would consitute no breaking change, since these are currently broken for non-cwd execution anyway.

Therefore I would prefer Option 2 (eventho handling would differ between "output files" and "hidden files"), but it's your call.

For consistency, I'd argue the debugging files should be treated the same as output files ext.Lcheck6_output.txt and ext.Lcheck6_output.FiniteDiff.txt in https://github.com/PrincetonUniversity/SPEC/blob/6d2693c9aa5a964146f1bdeaa2c09d0dde2619a9/src/dforce.f90#L1035-L1038

jonathanschilling commented 1 day ago

Hey Chris, I would actually support the point of Philipp of writing files next to the input files. Remember in the early days that SPEC required the input file to be located in the CWD as well, IIRC? I think it would be nicely consistent to have files be written next to the input file.