UFL

Remove set_foo functions in finiteelement.py

Bug #733774 reported by Garth Wells
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
UFL
Fix Released
Medium
Unassigned

Bug Description

UFL objects should all be immutable, and the the set_foo functions in finiteelement.py violate this.

Discussion from mailing list:

I don't have time now either, but I think it can be done easier:

Find all arguments and coefficients in form or expression.

Make new ones and make a dict mapping from old to new.

Use the existing function replace().

Martin
Den 12. mars 2011 11.44 skrev "Anders Logg" <email address hidden> følgende:
> On Sat, Mar 12, 2011 at 11:41:23AM +0100, Martin Sandve Alnæs wrote:
>> They should be removed. If used on an element used by an expression, subtle
>> incorrectness issues will appear, sometimes... Cache issues are only a symptom
>> here, the real problems will be wrong results with no crash.
>
> Yes I agree, no objections. The only problem is someone needs to write
> a transformer that replaces the element degree/shape throughout a form
> and I won't have time to dig into that just now.
>
> --
> Anders
>
>
>> Martin
>>
>> Den 12. mars 2011 11.36 skrev "Anders Logg" <email address hidden> f lgende:
>> > On Sat, Mar 12, 2011 at 11:28:15AM +0100, Martin Sandve Aln s wrote:
>> >> Changing repr is incorrect, and the bugs you're finding will never end.
>> >>
>> >> It is a fundamental design choice in UFL to make everything
>> >> immutable, and the repr strings can never change. This is a
>> >> critical property for lots of things to work, and it is assumed
>> >> all over the place. It cannot ever be changed.
>> >>
>> >> Instead of modifying an UFL object, always create a
>> >> new one with the changes you want. This is how every
>> >> algorithm in UFL works, and this is how every future
>> >> algorithm working on UFL objects must work.
>> >
>> > ok, good point.
>> >
>> > The reason for these changes are the two functions set_cell and
>> > set_degree in the finite element classes, which are used to set the
>> > cell and degree of an element when they are unspecified ("?"). They
>> > have been around for a long time.
>> >

description: updated
Changed in ufl:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Martin Sandve Alnæs (martinal) wrote :

It's not that simple...

To replace elements, we need to replace coefficients and arguments. In pure UFL this is probably not a problem. The replacement can use the replace function.

However, when interactions with PyDOLFIN are mixed in, I'm not sure how to solve this. The coefficient and argument objects will need to be new instances, so the multiple inheritance patterns in PyDOLFIN will cause problems if you want to keep the dolfin function instances.

Then again, why do you need to modify the functions in the first place? Why not just recreate the form with correct elements? I'm leaning towards claiming this is simply invalid use of UFL and needs to be solved another place.

Just so we're clear: I _will_ remove the set_foo functions before UFL 1.0, with or without a replacement feature. They're simply broken by design and can't stay.

Changed in ufl:
status: Confirmed → Fix Committed
Changed in ufl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.