Add a backup solution if unit cell is not defined/usable
Description of work In case no unit cell was defined for a system (or the cell is set to 0), calculates the maximum r in DistHistCutoffConfgurator from the span of atom coordinates.
This does not mean that the analysis results will be correct. For now, this PR will stop the GUI from crashing.
closes #516
Fixes
- Added a backup calculation of maximum r for DistHistCutoffConfigurator.
- Added a preview of the first unit cell in trajectory view.
To test Load a trajectory without a unit cell (Gaussian, xtb, Dmol3, ...?). Check that the GUI does not crash when Pair Distribution Function or Static Structure Factor are selected.
This PR has now reached the stage where:
- The GUI does not crash if the trajectory has no unit cell,
- The Molecular Viewer / Trajectory Info shows a preview of the first unit cell dimensions,
- Pair Distribution Function can be run when no unit cell is defined. It then outputs warnings every time a fake unit cell is constructed by DistanceHistogram,
- Static Structure Factor and Density will both fail if no unit cell is defined or the cell volume is 0.
- Plotter will not crash if a dataset consisting of NaN or Inf is loaded for plotting.
A separate problem has been discovered: in trajectories containing only a single atom of a specific element (and any number of other atoms), the total PDF will be NaN. Added as issue #520
Following your recommendation, different analysis types now fail if unit cell is not defined, but they recommend TrajectoryEditor as a backup solution, which allows adding a unit cell definition to a trajectory that did not have one.
TrajectoryEditor has been added, together with a new UnitCellConfigurator and Widget. It has its own unit tests, confirming that it can remove frames, remove atoms, change chemical elements and add a fixed unit cell to a trajectory.
I've loaded a trajectory without a unitcell and used the trajectory editor to add one. This seems to work fine but when I run the static structure factor or pair distribution function it doesn't seem to produce any results.
Also, the job crashes when using the atom selection with the trajectory editor. The following error logs are produced.
2024-08-07 10:17:50,500 - CRITICAL - process[28196] - IJob 418 - Job failed with traceback: Traceback (most recent call last):
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\Framework\Jobs\IJob.py", line 410, in run
IJob._runner[mode](self)
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\Framework\Jobs\IJob.py", line 253, in _run_singlecore
idx, result = self.run_step(index)
^^^^^^^^^^^^^^^^^^^^
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\Framework\Jobs\TrajectoryEditor.py", line 156, in run_step
com_conf = PeriodicRealConfiguration(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\MolecularDynamics\Configuration.py", line 215, in __init__
super(_PeriodicConfiguration, self).__init__(
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\MolecularDynamics\Configuration.py", line 59, in __init__
self[k] = np.array(v, dtype=float)
~~~~^^^
File "C:\Users\xcb63893\PycharmProjects\MDANSE\MDANSE\Src\MDANSE\MolecularDynamics\Configuration.py", line 111, in __setitem__
raise ValueError(
ValueError: Invalid item dimensions for velocities; a shape of (297, 3) was expected but data with shape of (300, 3) was provided.
Is it possible that the first problem was in fact not related to the cell definition, but instead this: https://github.com/ISISNeutronMuon/MDANSE/issues/520 ? As for the way the velocities are handled, thanks for spotting it. I will correct it.
With the current version, I was able to run TrajectoryEditor with atom selection, and run Static Structure Factor calculations on the resulting trajectory afterwards. If the trajectory you tried to analyse was one of those with a single atom of one element, please run the TrajectoryEditor on it again, this time setting a unit cell and removing that single atom. If you are able to run the Static Structure Factor on the resulting trajectory, then the other problem you faced was most likely the bug I mentioned above, and not a problem with the TrajectoryEditor itself.
Distinct van Hove function calculation now has a test for the presence of a valid unit cell. The job will fail if the unit cell is missing or set to zero size.
Looks good. I've tested it out and it all works as expected. I can approve and merge once the conflicts have been fixed.
The conflicts have been resolved. I also added the check for the valid unit cell to the self van Hove function, and another check to the DistHistCutoffConfigurator. I think it should be ready now.
The self van Hove function had the maximum distance cutoff switched off so that it doesn't have a maximum. It's similar to the situation for diffusion and mean square displacement. Can you revert that change?
I removed the unit cell check from the self van Hove function.