Project

General

Profile

Task #839

Improve OptionsIterator/OptionsGlobalProperties interface

Added by Teemu Murtola about 8 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
core library
Target version:
Difficulty:
uncategorized
Close

Description

Currently, the OptionsIterator class only provides access to features that are common to all options. Some hacks are already in place to be able to treat file name options specially. It would be good to make this more generic, and remove the hacks specific to file name options.

Similarly, OptionsGlobalProperties class is not generic, and needs to be changed every time a new property needs to be added. In addition, it introduces some "weak" cyclic dependencies, so it would be better to have a more generic mechanism. Quite a few of the use cases (or possibly even all) could be replaced with an OptionsIterator-based solution if it were more generic.

Both would require some sort of RTTI approach.


Related issues

Related to GROMACS - Task #708: Refactor options module to support adjusting option settings after creationClosed02/19/2011

Associated revisions

Revision e9b138dd (diff)
Added by Teemu Murtola almost 8 years ago

Pass settings object to initAnalysis().

Makes it easier to write analysis modules that need to access some
settings based on user input, and is also prerequisite for issue #839.
With the settings object, it is possible to pass additional information
to this method without changing the signature, simplifying maintenance.
In an earlier design, the settings object was available in all methods
through a separate accessor method, but the availability of the settings
became too restricted when this was refactored to make
TrajectoryAnalysisModule more interface-like.

Change-Id: If775db348354b6ba35c6d7cb0a10fe5057b6e1b2

Revision 10414111 (diff)
Added by Teemu Murtola almost 8 years ago

Make OptionsInfo more generic.

It is now possible to access information specific to a particular option
type from an OptionsVisitor. Removed hacks that were in place to treat
file name options specifically, since they can now be treated through
the generic framework.

Also added possibility to modify options using a non-const OptionInfo
object, plus visitor and iterator interfaces to use this. Not used yet
in any code, but will be in subsequent commits.

Part of issue #839.

Change-Id: If812252c73616a775ea5e2e13886a0a54db6ceaa

Revision ff7bfa6b (diff)
Added by Teemu Murtola almost 8 years ago

Remove SelectionOptionAdjuster.

There is no reason to have a separate SelectionOptionAdjuster now that
SelectionOptionInfo provides a more general mechanism to achieve the
same thing. Kept the SelectionOption::getAdjuster() method as the way
to obtain a SelectionOptionInfo object, but this can be replaced by a
more general mechanism if this functionality is needed for other option
types.

Related to issue #839.

Change-Id: Ief109c61e346cdfac2c665d7ed9a3002f80cc57f

Revision 5327e6a4 (diff)
Added by Teemu Murtola almost 8 years ago

Remove selection refs from OptionsGlobalProperties.

Instead of a global property, use the new modifying iterator interface
to pass the selection collection to each selection option.

Part of issue #839.

Change-Id: I727c1285a6f79783d7599fd25a8421e83609f27a

Revision d98556ab (diff)
Added by Teemu Murtola almost 8 years ago

Removed time unit from OptionsGlobalProperties.

Replaced the shared time unit value with a separate TimeUnitManager
class that provides basic functionality for time unit conversions, can
add an option for setting the time unit, and can scale time options
according to a time unit. Control flow should be easier to follow now
that the time options don't implicitly interact.

Part of issue #839.

Change-Id: Ib89459fae744a17e7d7c98a4d7e878b0c2dcf1ab

Revision b34fca3e (diff)
Added by Teemu Murtola almost 8 years ago

Remove rest of OptionsGlobalProperties.

Replaced plot type handling with a separate AnalysisDataPlotSettings
class, which is now declared in the same module where the actual
plotting is implemented.

Interaction with the existing output_env_t structure (that is needed for
passing to legacy methods) could be improved, but that is internal and
local to a few classes, so it's easy to fix later.

Also fixed a MSVC-specific warning in basicoptions.cpp that for some
reason appeared, although that part of the code was not touched.

Part of issue #839.

Change-Id: I5a18f5730f915578ff4661b48ec36b07d9cee35c

History

#1 Updated by Teemu Murtola almost 8 years ago

  • Status changed from New to In Progress
  • Assignee set to Teemu Murtola

The solution of completely removing OptionsGlobalProperties in favor of a iterator/visitor-based approach probably results in code that is easiest to understand.

#2 Updated by Teemu Murtola almost 8 years ago

  • Status changed from In Progress to Feedback wanted
  • Target version set to 5.0

The solution of completely removing OptionsGlobalProperties in favor of a iterator/visitor-based approach probably results in code that is easiest to understand.

Set of changes that does just that is now pending review in gerrit.

#3 Updated by Teemu Murtola almost 8 years ago

  • Status changed from Feedback wanted to Closed

All changes have been merged to master.

#4 Updated by Teemu Murtola almost 6 years ago

  • Project changed from Next-generation analysis tools to GROMACS
  • Category set to core library

#5 Updated by Teemu Murtola almost 6 years ago

  • Related to Task #708: Refactor options module to support adjusting option settings after creation added

Also available in: Atom PDF