Udef checks dependencies #1615
Udef checks dependencies #1615
Conversation
When delete a element that depens of a motor group a better message for the user is printed.
now it takes into account if the listeners are from an object instance
- Dereference weakrefs stored in PoolBaseChannel._pseudo_elements. - Fix PoolBaseChannel.remove_pseudo_element() - Fix docstring of PoolBaseChannel.get_pseudo_elements()
Performing more tests revealed that these references are not problematic in order to remove the pseudocounter core object correctly. Finally do not delete them.
Seems like PyTango/Tango keeps alive Device objects after deleting them (to be verified). These objects keep reference to the Sardana core objects, making it impossible to delete them and affecting the dependency relations. Delete reference to the motor_group in order to avoid this problem.
"head" kwarg is not used by the acquisition actions and it ends up as strong reference in the acquisition sub-actions. This leads to a cycle reference of the measurement group and the acquisition actions causing problems when discovering dependent elements when deleting an element.
Measurement group becomes an operator of the controller during an acquisition and remains as operator after. This creates an additional strong reference to the measurement group making it impossible to really delete causing troubles when discovering dependent elements on an element deletion. Use weakref.ref instead.
When deleting element, no dependent element can exist. First delete pseudo element and pseudo controller, and then the physical ones.
When deleting an element no dependent elements can exist. Delete the motor groups before the physical motors are deleted.
Pseudo counters must have cycle references with physical elements. This causes problems when deleting elements after prior deletion of pseudo counters. This was discovered with the following tests: - sardana/macroserver/macros/test/test_scanct.py::AscanctTest::test_ascanct_macro_runs_4 - sardana/macroserver/macros/test/test_scanct.py::AscanctTest::test_ascanct_macro_stops - sardana/macroserver/macros/test/test_scanct.py::A2scanctTest::test_a2scanct_macro_runs - sardana/macroserver/macros/test/test_scanct.py::MeshctTest::test_meshct_macro_runs - sardana/tango/pool/test/test_measurementgroup.py::TangoAcquisitionTestCase::test_meas_cont_acquisition_2 - sardana/tango/pool/test/test_measurementgroup.py::TangoAcquisitionTestCase::test_stop_meas_cont_acquisition_3 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_2 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_3 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_4 - sardana/taurus/core/tango/sardana/test/test_measgrpstress.py::TestStressMeasurementGroup::test_stress_count_6 Force gc.collect() to avoid this problem.
|
@dschick I think this is ready to be tested. |
|
I have tested this PR for #1658 . It works as expected and prevents the user from removing a measurement group dependent channel with udefelem. Thanks for the implementation! |
|
Its fine for me. Many thanks
reszelaz ***@***.***> schrieb am Mi., 14. Juli 2021, 11:29:
… Many thanks @yimli <https://github.com/yimli> for your test!
@dschick <https://github.com/dschick> do you also want to perform some
tests?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1615 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADJIQLHYKLGIDNLZ4FQKWO3TXVKJJANCNFSM463Z6QVA>
.
|
| @@ -557,6 +558,19 @@ def delete_element(self, name): | |||
| raise Exception("There is no element with name '%s'" % name) | |||
|
|
|||
| elem_type = elem.get_type() | |||
| dependent_elements = elem.get_dependent_elements() | |||
| if len(dependent_elements) > 0: | |||
| gc.collect() | |||
There was a problem hiding this comment.
perhaps a comment regarding why this is required here in the PR description?
|
I have tested Pseudomotors that have as dependent elements another Pseudomotors and it works! |
get_dependent_elements() returns only element name. Change the return value to return the dependent elements objects in order to extend use cases of this method e.g. allow to check the type of the returned dependent elements.
gc.collect() workaround is tried if any kind of dependent element resides in the pool. Call it only in the case of pseudo counters.
Disabled channels in the measurement group are considered as not members of the group. They neither appear in "element list" attribute nor the measurement group listens to their events. Hence it is possible to undefine an experimental channel which is disabled. This may lead to a situation when we try to enable again an undefined channel. Consider this case by not accepting a configuration which contains an undefined element. Also allow to acquire with a measurement group which contains and undefined and disabled channel.
PoolElement implements get_dependent_elements() and has_dependent_elements() while PoolBaseElement only implements get_dependent_elements(). Implement has_dependent_elements() also for the PoolBaseElement to be able to use both methods on any type of Sardana element.
cycle-reference may exist between an element and a traceback stored in SardanaValue or SardanaAttribute as a consequence of getting sys.exc_info(). Keeping a list of dependent elements in the scope of delete_element() makes it impossible to gc.collect() to collect the cycled objects. Use has_dependent_elements() in order to determine if there are dependent elements and then call the gc.collect(). Also, do not call it conditionally only for the pseudo counters but for any element type - the cycle may be created for between any alement and a traceback.
|
I have performed some more work on this and introduced the following changes. change return type of get_dependent_elements()I change the return value of Fix case with meas group with undefined disabled channelDisabled channels in the measurement group are considered as not members of the group. They neither appear in "element list" attribute nor the measurement group listens to their events. Hence it is possible to undefine an experimental channel that is disabled. This may lead to a situation when we try to enable again an undefined channel. Consider this case by not accepting a configuration that contains an undefined element. Also, allow to acquire with a measurement group that contains an undefined and disabled channel - see 6ac7aa6 Example: Fix
|
|
Many thanks! @reszelaz I just have a follow-up question about undefining disabled channel. It is reasonable that we should be able to undefine a disabled experimental channel through 'udefelem', but that still cause the expconf to report channel missing error if we open expconf after undefining a disabled channel. Maybe even if the channel is disabled, it is better to first manually remove it from expconf? I am not sure about this. |
|
Thanks for the tests with the expconf @yimli. I confirm what you observe. We get the following exception following the example from #1615 (comment) I will check if there is an easy way to prevent this exception in the expconf. |
sardana-org#1615 introduced bugs in expconf when a channel is undefined but still present in the measurement groups configurations. Fix it by disabling all cells of a disabled channels.
This pull request resolves #1586. Now a check for dependent elements is performed when an element is going to be undefined.
So far we tested it for Pseudocounters, Motorgroups, Pseudomotors and, Measurement Groups.
The remaining tests could be testing for Pseudomotors that have as dependent elements another Pseudomotors.
IMPORTANT: This PR introduces usage of
gc.collect()in Sardana. cycle-reference may exist between an element and a traceback stored inSardanaValueorSardanaAttributeas a consequence of gettingsys.exc_info()and storing a reference to it. Since dependent elements are determined by introspecting the listeners we need to attempt garbage collection of these cycled-referenced objects.