Energy load file only when necessary #331
Conversation
There was a problem hiding this comment.
This needs some deeper restructuring to correctly handle the exceptional case of getting passed an incorrect DatFile path in the rml file. Depending on the rework, the case where the tracer is already running and the DatFile is missing needs to be handled still (currently not handled).
| try | ||
| { | ||
| std::ifstream s(filename); | ||
| RAYX_VERB << filename; |
There was a problem hiding this comment.
"Reading in file: " << filename
| return true; | ||
|
|
||
| } catch (const std::exception& e) { | ||
| RAYX_D_ERR << "Exception caught while loading DatFile \"" << filename << "\": " << e.what(); |
There was a problem hiding this comment.
RAYX_VERB << "Dat-File with name: " .... << " could not be parsed."
There was a problem hiding this comment.
Exiting here is problematic, takes agency away from the caller of the function, even if it's in debug. RAYX_D_ERR should probably be an assertion in general now that we do not use glsl anymore
There was a problem hiding this comment.
Reviewing the other code I believe a boolean return value cannot handle this exceptional behavior. The stack is too deep to use return values and an exception should be thrown.
There was a problem hiding this comment.
On further review I believe that lazy loading the energy file during the compile call creates quite a few problems. A broken DatFile path in the rml file should already be found (but not handled) during the importBeamline(...) function call. rayx could then exit with the error message, that the file could not be found at the given path or it could not be parsed. rayx-ui could prompt the user with a pop-up with a similar/the same error message
|
|
||
| if (energyDistributionType == EnergyDistributionType::File) { | ||
| std::string filename = m_elementParameters["photonEnergyDistributionFile"].as_string(); | ||
|
|
||
| std::cout << std::filesystem::current_path() << std::endl; | ||
| DatFile df; | ||
| DatFile::load(filename, &df); |
There was a problem hiding this comment.
Result returned by DatFile::load is not handled here. What should getEnergyDistribution return when the path cannot be parsed. The stack seems to be quite deep at this point, so it's hard to imagine that return values can handle this. My first impression here would be to use an exception
fannyzotter
force-pushed
the
energyfile
branch
from
62e2e50 to
11de4ef
Compare
put an if condition for energyType Files
would that work for you?