Refactoring #1428

Feature #1534: === Parameter handling ===

Compactify code that handles parameters (use abstract mechanism instead of treating each single parameter explicitly)

Added by wuttke over 3 years ago. Updated almost 3 years ago.

Status:ArchivedStart date:10 May 2016
Priority:NormalDue date:
Assignee:wuttke% Done:

0%

Category:-
Target version:Sprint32

Description

Each parameter should have a default value and a unit physical dimension, assigned upon registration.

This will allow the following simplifications:
  • In PyGenVisitor::defineFormFactors, no more need handle each form factor separately and to dispatch parameter by parameter to printNm or printDegrees; instead we can just loop over parameters.
  • In GUIObjectBuilder::visit just call generic default method, as we are already doing in SamplePrintVisitor::print_default.
  • In FormFactorItems, move the 'addProperty' statements from the {FF}Item constructors to a loop in the parent FormFactorItem constructor.
  • In FormFactorItems, somehow get createFormFactor() done once for all form factors.

As for the GUI, see in a next step.

History

#1 Updated by wuttke over 3 years ago

  • Parent task set to #1290

#2 Updated by wuttke over 3 years ago

To automatically generate Python constructors,
  • either use named arguments
  • or make sure that parameters are ordered.

Currently, parameters are stored in an unordered map.

#3 Updated by pospelov over 3 years ago

Each parameter should have a default value

What does it mean? "new FormFactorSphere()" becomes valid and becomes equivalent of, for example, "new FormFactorSphere(5.0)" ?

#4 Updated by wuttke over 3 years ago

interesting consequence. why not?

#5 Updated by herck over 3 years ago

The {FF}Items need default values because the GUI needs to create them without user initialization. There is no need for this in the core.

#6 Updated by wuttke over 3 years ago

The very first question then: Is it actually appropriate for the GUI to assign default parameters to newly created items? Is this a deliberate choice for optimum user experience? Which alternatives have been considered?

#7 Updated by pospelov over 3 years ago

In GUI default values should be provided to reduce amount of clicks/typing. This allows to start a simulation with a few clicks after the start of the program. Also users often knows nothing about reasonable initial values (this is related for complex things (e.g. Itenrference Functions, but also for form factors)).

Core form factors should not have any default values. It's not their responsibility. If one need default FormFactorSphere, the corresponding Factory should be provided. This allows to have different factories/variants and so on.

#8 Updated by wuttke over 3 years ago

So yes, you want users to get default parametrized items. OK, I accept.

Core form factors should not have any default values.

Not yet convinced.

It's not their responsibility.

We are free to assign responsibilities to our creatures as we deem appropriate.

the corresponding Factory should be provided

Extra code? How many lines per form factor?

#9 Updated by herck over 3 years ago

It's not their responsibility.

We are free to assign responsibilities to our creatures as we deem appropriate.

Following the single responsibility principle, we are not free not add new ones. In this case, I feel it's even worse, since the reason for adding them comes from outside the library (GUI code).
The interface of core form factors (IFormFactor) is pretty minimal (evaluate, clone, ...) and serves only internal core logic. Default constructors/parameters are only justified for the sake of simplifying a refactoring of part of the GUI code. I strongly feel this should be handled by the GUI itself. We need to think first how to solve the underlying problem locally, i.e. in the GUI code.

#10 Updated by herck over 3 years ago

Maybe this issue should also be split: default values vs units

#11 Updated by wuttke over 3 years ago

So I withdraw default units from the current proposal. Are units acceptable for you?

#12 Updated by wuttke over 3 years ago

  • Subject changed from parameters should have default value and unit to Compactify code that handles parameters (use abstract mechanism instead of treating each single parameter explicitly)
  • Description updated (diff)

#13 Updated by wuttke over 3 years ago

  • Description updated (diff)

By convention, all lengths are in nm, all angles are internally in rad. Therefore, we only need the physical dimension, not the unit.

#14 Updated by wuttke over 3 years ago

  • Status changed from Rfc to Backlog

agreed in meeting today

#15 Updated by wuttke about 3 years ago

  • Parent task changed from #1290 to #1534

#16 Updated by wuttke about 3 years ago

  • Status changed from Backlog to Sprint
  • Assignee set to wuttke
  • Target version set to Sprint32

I started with simplifying the interface IParameterized (branch 'parunit') ...

#17 Updated by wuttke about 3 years ago

  • Status changed from Sprint to Resolved

Resolved as far as RealParameters are concerned.

Unifying the treatment of ICompositeSample's children and IParameterized's RealParameters must be addressed in a new issue, if at all.

#18 Updated by herck almost 3 years ago

  • Status changed from Resolved to Archived

Also available in: Atom PDF