Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default settings should be managed by the SimulationModule #29

Open
jgroehl opened this issue Apr 9, 2021 · 3 comments
Open

Default settings should be managed by the SimulationModule #29

jgroehl opened this issue Apr 9, 2021 · 3 comments
Assignees
Labels
feature New feature or request hacking week
Milestone

Comments

@jgroehl
Copy link
Collaborator

jgroehl commented Apr 9, 2021

Define abstract method in SimulatioModule class

@jgroehl
Copy link
Collaborator Author

jgroehl commented Apr 15, 2021

lets use this for the main documentation which parameters the adapter needs

@jgroehl jgroehl added the feature New feature or request label May 27, 2021
@jgroehl jgroehl added this to the Release 1.0.0 milestone May 27, 2021
@jgroehl jgroehl modified the milestones: Release 1.0.0, Release 2.0.0 Nov 19, 2021
@TomTomRixRix
Copy link
Collaborator

TomTomRixRix commented Jun 27, 2024

  • Set default component settings in adapters and global settings in PipelineModule by defining an abstract method get_default_settings() and overriding this in the individual adapters.
  • Document which Tags are used by each adapter and what the sensible defaults are
  • adjust the examples so that they don't need to specify all settings anymore
  • test if specified settings override the default settings
  • adjust available test if necessary

@TomTomRixRix
Copy link
Collaborator

In PR #358 I've now implemented a way to set default component settings for the adapters and default global settings. I think before proceeding further with the documentation, examples and tests it would make sense to have a meeting where we go through this list and check if all default values are sensible and if no default parameter was forgotten.

Furthermore three questions came up:

  1. Should processing components also have default settings?
  2. Would it make sense to treat all settings as a read-only configuration? Then we would have to find a new solution for the wavelength and simpa_output_path in the simulate method which are currently stored in the global settings during simulation. Passing them as an additional argument to the run method of the pipeline elements would be one option.
  3. Would it make sense to have a list of required (component) settings for each adapter which can be checked in the beginning of each adapter and a warning or error can be logged if something is missing. Then the user would have a better overview which of these required settings are already set by default and which they have to set manually. This should of course also be included into the documentation of each adapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request hacking week
Projects
None yet
Development

No branches or pull requests

4 participants