Path bug when supplying custom macro file with --vis_mac

[Former user]

First:

Using a vis_mac file that doesn't exist will cause BDSIM to use the default visualiser, and print a warning in the visualiser prompt. This is fine except when the visualiser seg faults, and you don't get to see such a message. This is very confusing, as it gives the impression that your alternative visualisation method (e.g. raytracer) is segfaulting. It should exit if it can't find the file.

Example:

$ bdsim --file=sm.gmad --vis_mac=doesn\'t_exist.mac

Secondly:

The path the user provides to --vis_mac is not a path relative to where BDSIM is called (which would be expected), but instead relative to where the GMAD file is. So if you have GMAD file in daughter directory and a .mac file in your current directory, to get it to use your vis_mac of choice, you actually have to do:

$ bdsim --file=daughter/sm.gmad --vis_mac=../vis.mac

which isn't intuitive.

[Jochem Snuverink]

Okay, I can take this one. The suggestions are good, but I wouldn't call this a bug though. First one is easy to fix, second might be more difficult, I'll have a look.

[Jochem Snuverink]

fix issue #196: more intuitive handling of vis_mac input file

→ <<cset f4162604d2b4>>

[Jochem Snuverink]

Okay, I fixed it as you proposed, but I realise now that setting the visMacroFileName in the options will be wrong now (since this should be relative to the gmad file).

[Jochem Snuverink]

By the way the other command line options that give input files, like --distrFile and possibly --recreate have the same issue.

I think we need to add some flags in options to distinguish between if a fileName was set from the command line or from the gmad input file. @nevay would you agree?

[Laurie Nevay]

So, the visualiser crashing is more a geant4 problem and a fairly niche problem for complicated geometry - that being said, it is coming up. We could change the behaviour to be that we exit instead of falling back to the default visualiser if the user one isn't found - that's quite simple.

In the case where the visualiser crashes, is the warning still not printed out beforehand? I imagine it must be printed before we start up the (default) visualiser?

[Former user]

The warning is printed in the visualiser, not the terminal, so if it segfaults before managing to open the visualiser you are never actually told that it didn't find your vis.mac.

--seedstatefile seems to segfault when it can't find the file, but if you give BDSIM a bogus --distrFile, it will carry on like nothing's happened... I think for "least surprise" having explicitly asked for a file and it can't find it then it shouldn't be OK with that.

[Jochem Snuverink]

@nevay I have fixed the exiting (and printing) in my commit.

@swalker92 distrFile is only sensitive when distrType is "userFile"

[Jochem Snuverink]

So in short the two problems described in the issue are fixed.

However, now the vis_mac path is relative to the execution path. This is wrong when it is set from within the gmad file. Usually the execution path and gmad file path were considered the same. However, if this isn't true (like in the issue), then a distinction needs to be made if the filename is set from the command line or from the gmad file.

[Laurie Nevay]

The executable options are processed by BDSExecOptions into an instance of parser/options.h. The parser is then used on the inputFileName member from these options to prepare a separate instance of parser/options.h (as well as all the other structures, ie parsing the input). The executable options are then amalgamated into the parser's set of options, when then becomes global constants. See the main and "AmalgamateOptions".

The parser has provision for building up file paths from the relative path of the parent gmad file and w.r.t. the execution location. This should prepare the file path correctly for ones set in the input gmad.

For exec options, we have functionality in BDSUtilities with GetFullPath. It could be we don't use or not correctly in BDSExecOptions. It looks like we don't use it at all.

Jochem, is there any overlap with the code you've added? Could be use BDS::GetFullPath?

[Jochem Snuverink]

GetFullPath searches relative to the input file .gmad. This we shouldn't use for exec options as pointed out correctly by Stuart.

For the parser options we do use GetFullPath inside bdsim, and hence by the mechanism you described also for the exec options (which is wrong).

What we can do is indeed already build up the absolute file name in BDSExecOptions when we process the command line (which we don't do now) and then leave the GMAD::Options and the rest as it is. I think that should cover all cases.

[Jochem Snuverink]

fix issue #196: build absolute paths in ExecOptions for input file names; put back GetFullPath in VisManager

→ <<cset 5621f7ba5482>>