Skip to content

Commit

Permalink
improved code and added test for next start index
Browse files Browse the repository at this point in the history
  • Loading branch information
modelonrobinandersson committed Nov 12, 2024
1 parent 81fe25e commit 51228a7
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 13 deletions.
24 changes: 16 additions & 8 deletions src/common/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -1630,14 +1630,24 @@ def get_variables_data(self,
for name in names:
trajectories[name] = self._get_variable_data_as_trajectory(name, time, start_index, stop_index)

largest_trajectory_length = -1
for v, t in trajectories.items():
largest_trajectory_length = max(largest_trajectory_length, len(t.x))
largest_trajectory_length = self._find_max_trajectory_length(trajectories)
new_start_index = (start_index + largest_trajectory_length) if trajectories else start_index

self._last_set_of_indices = (start_index, stop_index) # update them before we exit
return trajectories, new_start_index

def _find_max_trajectory_length(self, trajectories):
"""
Given a dict of trajectories, find the length of the largest trajectory
among the set of continuous variables. We disregard parameters/constants since they are not stored
with the same amount of data points as trajectories for continuous variables.
"""
length = 0

This comment has been minimized.

Copy link
@PeterMeisrimelModelon

PeterMeisrimelModelon Nov 13, 2024

Collaborator

Maybe rewrite using list-expression for performance? I.e.,

return max(0, max([len(traj) for v, traj in trajectories.items() if self.is_variable(v)])

This comment has been minimized.

Copy link
@modelonrobinandersson

modelonrobinandersson Nov 13, 2024

Author Collaborator

If trajectories={} this results in:
ValueError: max() arg is an empty sequence

This comment has been minimized.

Copy link
@PeterMeisrimelModelon

PeterMeisrimelModelon Nov 13, 2024

Collaborator

Right, how about

return max([0] + [len(traj) for v, traj in trajectories.items() if self.is_variable(v)])

I think the list comprehension should be more performant.

This comment has been minimized.

Copy link
@modelonrobinandersson

modelonrobinandersson Nov 13, 2024

Author Collaborator

See afba332

for var_name, trajectory in trajectories.items():
if self.is_variable(var_name): # since we only consider continuous variables
length = max(length, len(trajectory.x))
return length

def _calculate_events_and_steps(self, name):
if name in self._data_3:
return self._data_3[name]
Expand Down Expand Up @@ -1712,15 +1722,13 @@ def is_variable(self, name):
return True
elif '{}.'.format(DiagnosticsBase.calculated_diagnostics['nbr_state_limits_step']['name']) in name:
return True

variable_index = self.get_variable_index(name)
data_mat = self._dataInfo[0][variable_index]
if data_mat<1:
if data_mat < 1:
data_mat = 1

if data_mat == 1:
return False
else:
return True
return data_mat != 1

def is_negated(self, name):
"""
Expand Down
103 changes: 98 additions & 5 deletions tests/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,30 @@
from collections import OrderedDict

from pyfmi import testattr
from pyfmi.fmi import FMUException, FMUModelME2
from pyfmi.common.io import (ResultHandler, ResultDymolaTextual, ResultDymolaBinary, JIOError, ResultSizeError,
ResultHandlerCSV, ResultCSVTextual, ResultHandlerBinaryFile, ResultHandlerFile)
from pyfmi.common.io import get_result_handler
from pyfmi.common.diagnostics import DIAGNOSTICS_PREFIX, setup_diagnostics_variables
from pyfmi.fmi import (
FMUException,
FMUModelME2,
FMI2_PARAMETER,
FMI2_CONSTANT,
FMI2_LOCAL
)
from pyfmi.common.io import (
ResultHandler,
ResultDymolaTextual,
ResultDymolaBinary,
JIOError,
ResultSizeError,
ResultHandlerCSV,
ResultCSVTextual,
ResultHandlerBinaryFile,
ResultHandlerFile,
Trajectory,
get_result_handler
)
from pyfmi.common.diagnostics import (
DIAGNOSTICS_PREFIX,
setup_diagnostics_variables
)

import pyfmi.fmi as fmi
from pyfmi.tests.test_util import Dummy_FMUModelME1, Dummy_FMUModelME2, Dummy_FMUModelCS2
Expand Down Expand Up @@ -1695,6 +1714,80 @@ def test_csv_options_cs2(self):

class TestResultDymolaBinary:

def test_next_start_index(self):
"""
Test that calculation of the next start index works as expected.

This comment has been minimized.

Copy link
@PeterMeisrimelModelon

PeterMeisrimelModelon Nov 13, 2024

Collaborator

I think this could be a bit more specific, e.g.,

Test next start index calculation in the edge case of short trajectories and parameters

This test sets up a dummy FMU and dummy trajectories since we need
trajectories of uneven lengths.
"""
# Begin by setting up minimal required environment in order to perform the test
fmu = Dummy_FMUModelME2([], os.path.join(file_path, "files", "FMUs", "XML", "ME2.0", "CoupledClutches.fmu"),
_connect_dll=False)

result_handler = ResultHandlerBinaryFile(fmu)

opts = fmu.simulate_options()
opts["result_handling"] = "binary"
opts["result_handler"] = result_handler

fmu.setup_experiment()
fmu.initialize()
opts["initialize"] = False

result_handler.set_options(opts) # required in order to call simulation_start()
result_handler.initialize_complete()
result_handler.simulation_start()

This comment has been minimized.

Copy link
@chria

chria Nov 13, 2024

Collaborator

I think that we should test to retrieve a trajectory (using the get_variables_data method) before a call to the integration_point have been made.

This comment has been minimized.

Copy link
@modelonrobinandersson

modelonrobinandersson Nov 13, 2024

Author Collaborator

I have tried this but the ResultDymolaBinary constructor fails with an unexpected exception if I have not not invoked integration_point.


fmu.set('J4.phi', 1) # arbitrary
result_handler.integration_point()
rdb = ResultDymolaBinary(fmu.get_last_result_file(), allow_file_updates=True)

# Actual test starts below
vars_to_test = [
'J1.J', # this is a parameter
'clutch1.Backward' # this is a constant
]

# if this is not True, then the rest of test does not hold
assert vars_to_test[0] in result_handler.model.get_model_variables(causality = FMI2_PARAMETER).keys()

This comment has been minimized.

Copy link
@PeterMeisrimelModelon

PeterMeisrimelModelon Nov 13, 2024

Collaborator

Maybe more straight-forward to use fmu.get_variable_causality(...) ?

This comment has been minimized.

Copy link
@modelonrobinandersson

modelonrobinandersson Nov 13, 2024

Author Collaborator

I think it would be very equivalent. I wrote above because I read it as "verify that the variable is part of model parameters", and it reads well into the two lines below (for the constant and the state variable).
Let me know if you disagree, I have no strong preference.

This comment has been minimized.

Copy link
@PeterMeisrimelModelon

PeterMeisrimelModelon Nov 13, 2024

Collaborator

Current version is OK for me, I typically just like to avoid digging into attributes of objects to retrieve data.

assert vars_to_test[1] in result_handler.model.get_model_variables(variability = FMI2_CONSTANT).keys()
assert 'J4.phi' in result_handler.model.states.keys()


for v in vars_to_test:
trajectories1 = {
'J4.phi': Trajectory(np.array([]), np.array([])),
v: Trajectory(np.array([0]), np.array([1]))
}

trajectories2 = {
'J4.phi': Trajectory(np.array([0]), np.array([1])),
v: Trajectory(np.array([0, 1]), np.array([1, 1]))
}

trajectories3 = {
'J4.phi': Trajectory(np.array([0]), np.array([1])),
v: Trajectory(np.array([0]), np.array([1]))
}

trajectories4 = {
'J4.phi': Trajectory(np.array([0, 1]), np.array([1, 1])),
v: Trajectory(np.array([0]), np.array([1]))
}

trajectories5 = {
'J4.phi': Trajectory(np.array([0, 1, 2]), np.array([1, 1, 1])),
v: Trajectory(np.array([0]), np.array([1]))
}

assert rdb._find_max_trajectory_length(trajectories1) == 0
assert rdb._find_max_trajectory_length(trajectories2) == 1
assert rdb._find_max_trajectory_length(trajectories3) == 1
assert rdb._find_max_trajectory_length(trajectories4) == 2
assert rdb._find_max_trajectory_length(trajectories5) == 3

def _test_get_variables_data(self, dynamic_diagnostics: bool, nbr_of_calls: int, diag_data_ratio: int,
vars_to_test: list, stop_index_function: callable, result_file_name: str) -> dict:
"""
Expand Down

0 comments on commit 51228a7

Please sign in to comment.