- Member _gmx_sel_evaluate_subexpr (gmx_sel_evaluate_t *data, const gmx::SelectionTreeElementPointer &sel, gmx_ana_index_t *g)
- The call to gmx_ana_index_difference() can take quite a lot of unnecessary time if the subexpression is evaluated either several times for the same group or for completely distinct groups. However, in the majority of cases, these situations occur when _gmx_sel_evaluate_subexpr_staticeval() can be used, so this should not be a major problem.
- Member CHECK_SEL (sel)
- Get rid of this macro. It should now be possible to handle all errors using exceptions.
- File compiler.cpp
Better error handling and memory management in error situations. At least, the main compilation function leaves the selection collection in a bad state if an error occurs.
The memory usage could still be optimized. Use of memory pooling could still be extended, and a lot of redundant gmin/gmax data could be eliminated for complex arithmetic expressions.
- Page Custom selection methods
- The modifier handling could be made more flexible and more generic; the current implementation does not allow many things which would be possible with slight changes in the internals of the library.
- Member DIR_SEPARATOR
- Get rid of this (Redmine #950). It is not necessary for constructing paths on the systems that it currently supports, and is not reliable in parsing input paths either, since Windows needs to accept both instead of only DIR_SEPARATOR. At the very least, we should refactor the clients of this header so that they operate upon path objects rather than raw path strings.
- File domdec_network.h
- Wrap the raw dd_bcast in md.cpp into a higher-level function in the domdec module, then this file can be module-internal.
- File evaluate.cpp
- One of the major bottlenecks for selection performance is that all the evaluation is carried out for atoms. There are several cases when the evaluation could be done for residues or molecules instead, including keywords that select by residue and cases where residue centers are used as reference positions. Implementing this would require a mechanism for recognizing whether something can be evaluated by residue/molecule instead by atom, and converting selections by residue/molecule into selections by atom when necessary.
- Member exist_output_file (const char *fnm_cp, int nfile, const t_filenm fnm[])
- This could be implemented sanely with a for loop.
- Member fit_acf (int ncorr, int fitfn, const gmx_output_env_t *oenv, gmx_bool bVerbose, real tbeginfit, real tendfit, real dt, real c1[], real *fit)
- check parameters
- Member ftype_is_bonded_potential (int ftype)
- This function could go away when idef is not a big bucket of everything.
- Class gmx::AbstractAnalysisArrayData
- Add support for multiple data sets.
- Class gmx::AbstractAnalysisData
- Improve the exception-handling semantics. In most cases, it doesn't make much sense to continue data processing after one module fails, but having the alternative would not hurt.
- Member gmx::AbstractAnalysisData::addColumnModule (int col, int span, AnalysisDataModulePointer module)
This method doesn't currently work in all cases with multipoint data or with multiple data sets. In particular, if the added module requests storage and uses getDataFrame(), it will behave unpredictably (most likely asserts).
Generalize this method to multiple data sets (e.g., for adding modules that only process a single data set).
- Member gmx::AbstractAnalysisData::applyModule (IAnalysisDataModule *module)
- Currently, this method may not work correctly if
module
requests storage (addModule() has the same problem if called after data is started).
- Member gmx::AbstractOptionStorage::processSet ()=0
- Improve the call semantics.
- Class gmx::AnalysisData
- Parallel implementation is not complete.
- Member gmx::AnalysisDataPlotSettings::plotFormat () const
- Use a proper enum.
- Class gmx::AnalysisDataStorage
- Proper multi-threaded implementation.
- Class gmx::AnalysisNeighborhood
- Generalize the exclusion machinery to make it easier to use for other cases than atom-atom exclusions from the topology.
- Class gmx::AnalysisNeighborhoodSearch
Make it such that reset() is not necessary to call in code that repeatedly assigns the result of AnalysisNeighborhood::initSearch() to the same variable (see sm_distance.cpp).
Consider merging nearestPoint() and minimumDistance() by adding the distance to AnalysisNeighborhoodPair.
- Class gmx::ArrayRef< T >
- This class is not complete. At least, it should be possible to convert an ArrayRef to a ConstArrayRef. There are likely also methods missing (not required for current usage).
- Member gmx::DataFileInfo::bFromDefaultDir
- Consider replacing with an enum that identifies the source (current dir, GMXLIB, default).
- Class gmx::ExceptionInitializer
- With the exception of the reason string, information added with this class is not currently accessible through any public API, except for calling printFatalErrorMessage(), formatExceptionMessageToString() or formatExceptionMessageToFile(). This is not implemented as there is not yet need for it, and it is not clear what would be the best alternative for the access. It should be possible to refactor the internal implementation to suit the needs of such external access without requiring changes in code that throws these exceptions.
- Member gmx::FileNameOption::libraryFile (bool bLibrary=true)
- Currently, this flag only affects the help output. Callers must take care themselves to actually search the file in the library directories. It would be nicer to do this searching within the file name option implementation.
- Class gmx::FileNameOptionManager
- Most of the functionality in this class is specific to command line parsing, so it would be cleaner to replace this with an interface, and have the actual code in the
commandline
module.
- Member gmx::GromacsException::prependContext (const std::string &context)
- The added information is currently not accessible through what(), nor through any other means except for calling printFatalErrorMessage(), formatExceptionMessageToString() or formatExceptionMessageToFile(). See ExceptionInitializer for more discussion.
- Member gmx::HelpWriterContext::setReplacement (const std::string &search, const std::string &replace)
- Improve semantics if the same
search
item is set multiple times.
- Member gmx::internal::AnalysisDataStorageImpl::needStorage () const
- This could be extended to non-multipoint data as well.
- Member gmx::internal::AnalysisDataStorageImpl::pendingLimit_
- Get rid of this alltogether, as it is no longer used much.
- Member gmx::OptionFlag
- The flags related to default values are confusing, consider reorganizing them.
- Member gmx::Regex::Regex (const char *value)
- Consider whether some other exception type would be better.
- Member gmx::Selection::setEvaluateVelocities (bool bEnabled)
- Implement it such that in the above case, hasVelocities() will return false for such frames.
- Class gmx::SelectionOption
Support for specifying that an option accepts, e.g., two to four selections. Currently, only a fixed count or any number of selections is possible. In addition to allowing this in OptionTemplate, also SelectionOptionManager needs to be updated.
- Member gmx::SurfaceAreaCalculator::calculate (const rvec *x, const t_pbc *pbc, int nat, int index[], int flags, real *area, real *volume, real **at_area, real **lidots, int *n_dots) const
- Make the output options more C++-like, in particular for the array outputs.
- Class gmx::test::AbstractTrajectoryAnalysisModuleTestFixture
- Adding facilities to AnalysisData to check whether there are any output modules attached to the data object (directly or indirectly), marking the mocks as output modules, and using these checks in the tools instead of or in addition to the output file presence would be a superior. Also, the full file names should be deducible from the options.
- Class gmx::test::AnalysisDataTestFixture
- Support for arbitrary AnalysisDataValues (errors and missing values).
- Class gmx::test::FloatingPointTolerance
- The factory methods that take ULP difference could be better formulated as methods that take the acceptable number of incorrect bits and/or the number of accurate bits.
- Member gmx::test::IntegrationTestFixture::redirectStderrToDevNull ()
- Implement this when the output routines are sufficiently modular to permit it to work.
- Member gmx::test::IntegrationTestFixture::redirectStdoutToDevNull ()
- Implement this when the output routines are sufficiently modular to permit it to work.
- Class gmx::test::SettleTest
This also tests that if the calling code requires velocities and virial updates, that those outputs do change, but does not test that those changes are correct.
Only no-PBC and cubic-PBC are tested here, but the correct function of the SIMD version of set_pbx_auic in all cases should be tested elsewhere.
- Class gmx::test::TerminationHelper
- This approach is not very elegant, but "stuff doesn't
segfault or give a fatal error" is a useful result. We can improve it when we can mock out more do_md() functionality. Before that, we'd probably prefer not to run this test case in per-patchset verification, but this is the best we can do for now.
- Member gmx::test::throwIfNonEmptyAndOnlyWhitespace (const std::string &s, const char *id)
- Eliminate this limitation of TinyXML2. See e.g. https://github.com/leethomason/tinyxml2/issues/432
- Member gmx::TextWriter::wrapperSettings ()
- Wrapping is not currently implemented for code that writes partial lines with writeString().
- Class gmx::TimeUnitManager
This class is independent of the options implementation. To ease reuse, it could be moved to the utility module, and only TimeUnitBehavior left here.
- Class gmx::TrajectoryAnalysisSettings
- Remove plain flags from the public interface.
- File grompp.cpp
- Refactor SimulationRunner to split off SimulationPreparer, so that integration tests of grompp can stand apart from tests of mdrun.
- File handlerestart.cpp
- Clean up the error-prone logic here. Add doxygen.
- File handlerestart.h
- There may be other code in runner.cpp etc. that can usefully live here
- File indexutil.cpp
- Tests for other functions, at least the set operations.
- File invertmatrix.cpp
- Test error conditions when they throw exceptions
- File mempool.h
- Document these functions.
- Group module_domdec
- Get domdec stuff out of mdtypes/commrec.h
- File multisimtest.cpp
- Test mdrun -multidir also
- File nbsearch.cpp
- Increase coverage of these tests for different corner cases: other PBC cases than full 3D, large cutoffs (larger than half the box size), etc. At least some of these probably don't work correctly.
- File nbsearch.cpp
- The grid implementation could still be optimized in several different ways:
- Pruning grid cells from the search list if they are completely outside the sphere that is being considered.
- A better heuristic could be added for falling back to simple loops for a small number of reference particles.
- A better heuristic for selecting the grid size.
- A multi-level grid implementation could be used to be able to use small grids for short cutoffs with very inhomogeneous particle distributions without a memory cost.
- Member ocl_pmalloc (void **h_ptr, size_t nbytes)
- This function should allocate page-locked memory to help reduce D2H and H2D transfer times, similar with pmalloc from pmalloc_cuda.cu.
- Member pbc_rvec_sub (const t_pbc *pbc, const rvec xi, const rvec xj, rvec dx)
- This kind of code appears in many places. Consolidate it
- File poscalc.cpp
There is probably some room for optimization in the calculation of positions with bases. In particular, the current implementation may do a lot of unnecessary copying. The interface would need to be changed to make it possible to use the same output positions for several calculations.
The current algorithm for setting up base calculations could be improved in cases when there are calculations that cannot use a common base but still overlap partially (e.g., with three calculations A, B, and C such that A could use both B and C as a base, but B and C cannot use the same base). Setting up the bases in an optimal manner in every possible situation can be quite difficult unless several bases are allowed for one calculation, but better heuristics could probably be implemented. For best results, the setup should probably be postponed (at least partially) to gmx_ana_poscalc_init_eval().
- Member read_checkpoint_data (const char *filename, int *simulation_part, t_commrec *cr, gmx_bool bTryToAppendFiles, int nfile, const t_filenm fnm[], const char *part_suffix, gmx_bool *bAddPart, gmx_bool *bDoAppendFiles)
- Clean this up (next patch)
- Member SEL_ALLOCVAL
- This flag overlaps with the function of
v.nalloc
field, and could probably be removed, making memory management simpler. Currently, the v.nalloc
field is not kept up-to-date in all cases when this flag is changed and is used in places where this flag is not, so this would require a careful investigation of the selection code.
- Page Selection compilation
- Some combinations of method parameter flags are not yet properly treated by the compiler or the evaluation functions in evaluate.cpp. All the ones used by currently implemented methods should work, but new combinations might not.
- Member set_over_alloc_dd (gmx_bool set)
- This is mdrun-specific, so it might be better to put this and over_alloc_dd() much higher up.
- File simulationsignal.cpp
- Move this to mdrunutility module alongside gathering multi-simulation communication infrastructure there.
- File simulationsignal.h
- Move this to mdrunutility module alongside gathering multi-simulation communication infrastructure there.
- File sm_insolidangle.cpp
The implementation could be optimized quite a bit.
Move the covered fraction stuff somewhere else and make it more generic (along the lines it is handled in selection.h and trajana.h in the old C API).
- File snprintf.h
- When all callers of snprintf compile as C++, perhaps use gmx::formatString() everywhere instead of snprintf.
- File testasserts.h
The implementation is somewhat ugly, and accesses some Google Test internals. Could be nice to clean it up a bit.