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

Python Binding: inputParameters dict is shared between multiple calls to evaluate #32

Closed
Latios96 opened this issue May 27, 2024 · 0 comments

Comments

@Latios96
Copy link
Contributor

Latios96 commented May 27, 2024

Hi,

I noticed that if no inputParameters dict is supplied to flip.evalute via the Python bindings, the default argument is used. Since the default argument only exists one time in memory and is mutated, this leads to unexpected results for subsequent calls.

Steps to reproduce:

  1. Download the attached images and Python Script flip-reproduce.zip
  2. Execute python reproduce.py. The script calls flip.evaluate from Python with no custom input parameters. For the first call, Flip will calculate start/stop exposure (in this case nan) and will store them in the inputParameters before returning them. The second flip does not supply its own inputParameters, but since the default argument was mutated in the previous call, the values from the previous call are used. This leads to different results:

Two flip calls:

python reproduce.py
0.0 {'ppd': 67.02064514160156, 'startExposure': nan, 'stopExposure': nan, 'numExposures': 2, 'tonemapper': 'aces'}
0.0 {'ppd': 67.02064514160156, 'startExposure': nan, 'stopExposure': nan, 'numExposures': 2, 'tonemapper': 'aces'}

Skipping the first flip call:

python reproduce.py 1
0.07767405360937119 {'ppd': 67.02064514160156, 'startExposure': -4.310622692108154, 'stopExposure': 8.824731826782227, 'numExposures': 14, 'tonemapper': 'aces'}

Expected behavior
When not providing own inputParamaters, all calls to flip.evaluate should use defaults.
Flip should not reuse the computed start/stop exposure and other parameters.

Workaround
There is a workaround by passing an empty parameters dict to flip.evaluate in Python (see also workaround.py). This works as expected:

python reproduce_with_workaround.py
0.0 {'ppd': 67.02064514160156, 'startExposure': nan, 'stopExposure': nan, 'numExposures': 2, 'tonemapper': 'aces'}
0.07767405360937119 {'ppd': 67.02064514160156, 'startExposure': -4.310622692108154, 'stopExposure': 8.824731826782227, 'numExposures': 14, 'tonemapper': 'aces'}

I filed a PR with a proposed fix: #33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants