FeatureIDE / FeatureIDE

An extensible framework for feature-oriented software development
https://featureide.github.io/
GNU Lesser General Public License v3.0
129 stars 97 forks source link

Wrong DIMACS export for some feature models #1487

Closed st-vi closed 1 day ago

st-vi commented 3 weeks ago

Prerequisitives

Issue description

Exporting a UVL featuremodel to dimacs results in a wrong cnf. Example: The feature model

features
    "__Root__"
        optional
            CONFIG_DEFAULT_SETFONT_DIR
            CONFIG_SETFONT

results in the dimacs

c 1 __Root__
c 2 CONFIG_DEFAULT_SETFONT_DIR
c 3 CONFIG_SETFONT
p cnf 3 0

which is wrong, since the root feature is not forced to be true here. When we remove the underscores from the root feature name, everything works as expected:

features
    "Root"
        optional
            CONFIG_DEFAULT_SETFONT_DIR
            CONFIG_SETFONT

results in the dimacs

c 1 Root
c 2 CONFIG_DEFAULT_SETFONT_DIR
c 3 CONFIG_SETFONT
p cnf 3 3
1 0
1 -2 0
1 -3 0

which is correct. A common feature model with this issue is busybox (https://github.com/Universal-Variability-Language/uvl-models/blob/main/Feature_Models/Operating_Systems/BusyBox/busybox_2010-05-02_14-17-07.uvl).

h3ssto commented 3 weeks ago

Can confirm this issue. Does this also affect mandatory children (which would not be core, if the mandatory root feature is enforced)? This could threaten the validity of previous evaluation results.

skrieter commented 1 week ago

This is not a bug, but intented behavior. The dummy root feature is not written to the dimacs file to avoid unecessary bloating. You can disable this behavior in the FeatureIDE preference page by unchecking the option "Omit artifical root feature when saving DIMACS file".

SundermannC commented 1 day ago

I understand the automatic discard, but the resulting number of valid configurations is wrong, because the output does not remove Root but instead considers it a fully optional feature.

One fix could be to change p cnf 3 0 to p cnf 2 0.

This problem is also relevant because feature models computed from Tobias Pett (https://github.com/PettTo/Measuring-Stability-of-Configuration-Sampling) also have this artificial root name. When we convert those feature models we get faulty model counts.

skrieter commented 1 day ago

Ah, yes you are right. Now it is fixed.