Fix filepath handling - Githubissues

missing-user commented 1 week ago

Solves issue https://github.com/PrincetonUniversity/SPEC/issues/206 related to running SPEC from a different working directory than the input file.

Currently writes both output files and hidden files to the same directory as the input file.

The most interesting change is in the first commit, where we define the filepaths for the output files ext and the filepath for the hidden files hiddenext (derivative, hessian, ...)

smiet commented 1 week ago

Could we actually make the gradient files not be hidden? I don't think it makes much sense to do so and I actually find it very frustrating. It makes the run result stateful, but then hides the state.

About the writing to the folder where the input file is located, I agree that it is the best choice given that it doesn't break existing functionality.

I'll work on a little fix for simsopt so simsopt copies the spec input file to the pwd, to avoid clobbering issues

missing-user commented 1 week ago

Nice, glad we are all on the same page :)

Simsopt already has an optional argument to copy the files to pwd when creating the freeboundary default file, presumably because the hidden file handling was causing issues.

mhd.Spec.default_freeboundary(copy_to_pwd=True)

missing-user commented 1 week ago

List of all hidden files (could we add this table of output files to the documentation? I added the missing ones to grp_output for now in the readme fix branch):

Extension	Description	Originating Fortran File
`.GF.ev`	The eigenvalues and eigenvectors of the Hessian	`hesian.f90`
`.GF.ma`		`hesian.f90`
`.sp.DF`	Derivative matrix	`newton.f90`
`.sp.A`	Initial guess for vector potential	`ra00a.f90`

Do you @smiet think all of them should be "regular output files" then? I assume the reasoning was to limit how much the workspace is cluttered by each SPEC run. Also all hidden files are conditionally toggled on and off by some flags, while the regular output files are always generated by a SPEC run.

Once again this would be a breaking change tho, and requires an update to the simsopt Spec wrapper, which accesses .sp.DF and all other third party CI.

smiet commented 1 week ago

@abaillod, @jons-pf, @zhucaoxiang , what do you think about making these files visible?

We should probably move this discussion to an issue, @abaillod can of you take care of that? (Or shoot down my suggestion?) For now the fix solves the issue and doesn't change existing functionality.

Can you also trigger the tests and merge if all are successful?

@missing-user thank you so much for your work. I'm going to be traveling in the coming week so I won't be super responsive, my apologies for that

jons-pf commented 1 week ago

I would be in favour of making all produced files visible, because I have struggled myself in the early days with not being aware of them. One consideration to keep in mind in this regard is that users might have lots of files using the old naming scheme on disk (thinking mainly of @SRHudson, but potentially also others) - I would propose to make the writing use only the new scheme, but additionally (for now) keep the reading also backwards-compatible.

missing-user commented 5 days ago

I changed hidden_ext to a function, because the simsopt and py_spec wrapper were modifying allglobal.ext directly sometimes and they would go out of sync.

zhucaoxiang commented 4 days ago

@abaillod, @jons-pf, @zhucaoxiang , what do you think about making these files visible?

We should probably move this discussion to an issue, @abaillod can of you take care of that? (Or shoot down my suggestion?) For now the fix solves the issue and doesn't change existing functionality.

Can you also trigger the tests and merge if all are successful?

@missing-user thank you so much for your work. I'm going to be traveling in the coming week so I won't be super responsive, my apologies for that

@smiet I am okay with making these files visible.

PrincetonUniversity / SPEC

Fix filepath handling #208