feat(model): Add model values to represent a numeric value computed from a model - #185
Conversation
6f3123a to
67562b9
Compare
[Maven Build Status]📑 Commit: 📦 Download artifact: Generator.jar |
pyrollo
left a comment
There was a problem hiding this comment.
Quelques commentaires, plutôt sur le fond.
En fait, j'ai l'impression qu'on peut pousser le truc beaucoup plus loin. Evidemment, on ne va pas avoir le temps tout de suite.
En revanche, ça serait pas mal qu'on se cale un moment pour brainstormer là dessus, histoire de voir ce qui est réaliste à moyen terme et de faire en sorte d'aller doucement vers ça sans se bloquer.
En gros, savoir où on s'arrêtera, où on s'arrête uajourd'hui et savoir à quoi ça pourrait ressembler pour être sûr d'aller dans le bon sens.
67562b9 to
46d9c2e
Compare
pyrollo
left a comment
There was a problem hiding this comment.
Revue partielle, je continuerai mercredi.
Je pense qu'il faut supprimer le raccourci "XXX" pour prendre la valeur d'une métadata et ne garder que "metadata: XXX" (on sait déjà que ça va nous obliger à revoir le paramétrage plus tard).
Autre chose : les values sont dans models/values mais on sait aussi déjà que ça va se généraliser. Peut être peuvent-elles déjà être placées dans generator/values.
C'est dommage que tous les opérateurs ne puissent rentrer dans Binary/Unary operator.
pyrollo
left a comment
There was a problem hiding this comment.
J'ai fini la partie main, reste les tests.
J'ai clos un certain nombre de commentaires mais il reste des trucs à revoir.
12d3a13 to
0821695
Compare
0821695 to
18514aa
Compare
Changes
This PR brings model values, which are operators to compute numerical values from models, and integrate them into the codebase.
The main commit is based on the PR #130 from the FDS2025 code-sprint.
Feature description
This PR contains a total of 6 commits each with a relatively well defined scope. I strongly suggest reviewing them separately, in the order described below.
feat(model): Add model values to represent a numeric value computed from a model
The first commit introduces model values, with functional classes, parameters, documentation and unit tests. Functionally, a model value is a function accepting a model and producing an optional double value. It may or may not not use the model, and may or may not rely on other model values. Not every value exists for every model: it may return an empty optional, signaling an "absent" value. It forces every model values to explicitly encode their absent value policy, while still being concise.
Currently, model values only supports numeric values. In the future, this limitation might get removed to allow any type of value. It raises questions about current operators (how to compute the product of strings?), and there will probably be some joint work with heightmaps (that could become datamaps): it will then be the good time to refactor both heightmaps and model values to share their operators.
See
docs/usage/parameters/ModelValues.mdfor full parameters documentation.feat(tasks): Improve existing tasks to accept model values
The second commit improves the existing task able to fill between a heightmap and a metadata value to make it accept a model value instead. Parameters, documentation and unit tests are updated accordingly.
The next two commits are where things begin to go off-road. They both are motivated by their following one, so let's unwrap it starting with the fifth commit.
feat(filters): Improve model filters using model values
This fifth commit integrates model values into model filters. The two concepts are highly intertwined by nature, both taking a model to produce a well-defined result (boolean or numerical value). A new model filter now operates on a model value:
ModelFilterOnValue. It is heavily inspired by its equivalent for metadata,ModelFilterOnMetadataValue.From this functional filter are derived several filter parameters, including "has", "equals" and existing "lowerThan/greaterThan". Existing "has" and "equals" for metadata remains necessary when working with non-numeric values until model values are able to represent them.
However, model values parameters take the generation object as context for their creation, to allow implementations needing special values to exists. This is the case for the random model value needing the generation seed. We might also have special model values referring to generation constants such as the scale (not sure). To have model values in model filters, the latter thus also needs the generation context to be created. This cascades to model selection and post-processors params, leading to the fourth commit.
refactor(params): Pass generation context to params create method
This fourth commit refactors parameters creation to add the generation context object to the
createmethods of all model filters, selection and post-processors parameters.While we originally voted that out a the early stage of the project, I invite us to reconsider it through this PR, especially now that we are really close to upgrading to Java 25, which will bring scoped values, which we already decided to adopt, crossing the main barrier of "unexpected/shadowed dependency" we believed in before. While it was a good idea to try to explicitly limit which object each parameters had to create itself, is now becomes a limitation with our parameters-driven philosophy. To ensure maximum flexibility, each object must have what it needs at his disposal, without needing to add that specific requirement to all objects of its kind. Because the generation object holds all the context usually needed, it seems like the most logical thing to have.
Later on, we might decide to remove this parameter and instead provide its value using a scoped value, to reduce boilerplate code. Especially for unit tests, which is the reason of the third commit.
test(generation): Add TestingGeneration class to simplify tests
This third commit aims at simplifying unit tests when a generation object is needed. A new
TestingGenerationclass is introduced, with an easy-to-(not-)use instanceTestingGeneration.UNUSED.Several tests benefit from this new class centralizing all the arguments of its lengthy constructor. This also simplifies future updates of that constructor, by having a centralized location where it is used in unit tests.
feat(post-processor): Add post-processor to set a metadata value from a model value
The sixth and last commit adds a new post-processor, inspired by the "copy" post-processor, but instead defining a model value into a metadata. For the same reason as cited above, it does not replace it completely, until model values are no longer numerical-only.
This new "set" post-processor can be used both for optimization purpose (pre-compute a model value once for each model and use it multiple times later through its metadata), and for dynamic default numerical value (those value is computed from other metadata values, when present).
Showcase
Many examples can be found in the updated documentations files, especially
ModelValues.mdandModelSelection.md.An upcoming usage will be to compute the height of OSM buildings from the number of floors tags, which could translate to something like:
Note that in this example, the vertical scale is expressed as a YAML reference to an hypothetical anchor that would be placed on the actual scale parameter definition. This would need to adjust our helper script to concatenate the place parameters before the process parameters, which should be acceptable. Another option would be to introduce a special model value, that would be a kind of "generation constant" for the vertical scale. Both options sounds possible, although I personally prefer the first one.
Reason
The great flexibility behind readable heightmaps was missing on metadata values. This concept was approached twice, by 2 different developers, leading to surprisingly similar results! Once on the
pyr/rdrvaluesbranch, and especially this commit e61c2e0 from December 5, 2024, and a second time on thefds/mainbranch, and especially this commit 30bb861 from September 22, 2025.TODOs
Probable future works include:
Self-checks
/docsfolder has been updatedexamples/work the same (or have been adapted if subject to changes in this PR)