dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

On 22 November 2011 22:05, Anders Logg <logg@xxxxxxxxx> wrote:
> On Tue, Nov 22, 2011 at 10:03:19PM +0000, Garth N. Wells wrote:
>> On 22 November 2011 21:58, Anders Logg <logg@xxxxxxxxx> wrote:
>> > On Tue, Nov 22, 2011 at 09:53:47PM +0000, Garth N. Wells wrote:
>> >> On 22 November 2011 21:50, Anders Logg <logg@xxxxxxxxx> wrote:
>> >> > On Tue, Nov 22, 2011 at 09:33:25PM +0000, Garth N. Wells wrote:
>> >> >> On 22 November 2011 21:30, Anders Logg <logg@xxxxxxxxx> wrote:
>> >> >> > On Tue, Nov 22, 2011 at 10:16:54PM +0100, Marie E. Rognes wrote:
>> >> >> >> On 11/22/2011 09:55 PM, Anders Logg wrote:
>> >> >> >> >On Tue, Nov 22, 2011 at 08:45:30PM +0000, Garth N. Wells wrote:
>> >> >> >> >>
>> >> >> >> >>On 21 Nov 2011, at 21:53, "Marie E. Rognes"<meg@xxxxxxxxx>  wrote:
>> >> >> >> >>
>> >> >> >> >>>
>> >> >> >> >>>On 21. nov. 2011, at 21:52, Anders Logg<logg@xxxxxxxxx>  wrote:
>> >> >> >> >>>
>> >> >> >> >>>>On Mon, Nov 21, 2011 at 08:46:13PM +0000, Garth N. Wells wrote:
>> >> >> >> >>>>>On 21 November 2011 13:07, Anders Logg<logg@xxxxxxxxx>  wrote:
>> >> >> >> >>>>>>On Sun, Nov 20, 2011 at 11:55:43PM +0100, Anders Logg wrote:
>> >> >> >> >>>>>>>On Sun, Nov 20, 2011 at 11:49:42PM +0100, Marie E. Rognes wrote:
>> >> >> >> >>>>>>>>
>> >> >> >> >>>>>>>>On 20. nov. 2011, at 23:31, Anders Logg<logg@xxxxxxxxx>  wrote:
>> >> >> >> >>>>>>>>
>> >> >> >> >>>>>>>>>Is anyone using the Function constructor that takes a vector as input
>> >> >> >> >>>>>>>>>argument?
>> >> >> >> >>>>>>>>>
>> >> >> >> >>>>>>>>>Function u(V, x);
>> >> >> >> >>>>>>>>>
>> >> >> >> >>>>>>>>Yes.
>> >> >> >> >>>>>>>Does it work? In parallel?
>> >> >> >> >>>>>>>
>> >> >> >> >>>>>>>Does it not work to instead use
>> >> >> >> >>>>>>>
>> >> >> >> >>>>>>>  x = u.vector()
>> >> >> >> >>>>>>>
>> >> >> >> >>>>>>>?
>> >> >> >> >>>>>>>
>> >> >> >> >>>>>>>If you need it, we should keep it but add an error message that it
>> >> >> >> >>>>>>>doesn't work in parallel, unless it does...
>> >> >> >> >>>>>>Any more input on this? There are several options:
>> >> >> >> >>>>>>
>> >> >> >> >>>>>>1. Remove this constructor
>> >> >> >> >>>>>>
>> >> >> >> >>>>>>2. Throw an error when running in parallel
>> >> >> >> >>>>>>
>> >> >> >> >>>>>>3. Check that the input vector makes sense
>> >> >> >> >>>>>>
>> >> >> >> >>>>>>The last one is problematic since I don't see an easy way to perform
>> >> >> >> >>>>>>the check, other than calling get_local and having it fail.
>> >> >> >> >>>>>>
>> >> >> >> >>>>>I haven't heard any reason why it can't be removed. We may need to fix
>> >> >> >> >>>>>assignment (re earlier discussion on assign) to just copy values and
>> >> >> >> >>>>>not the whole object so that a user can get the vector and then assign
>> >> >> >> >>>>>values to it without messing up the ghosting.
>> >> >> >> >>>>Sounds good, but I want to wait for Marie to comment before I remove
>> >> >> >> >>>>it. She is using it.
>> >> >> >> >>>>
>> >> >> >> >>>>Marie? Does it work for you to use x = u.vector()?
>> >> >> >> >>>>
>> >> >> >> >>>Probably. However removing the constructor would be changing parts of the basic interface, which I think is a bad idea.
>> >> >> >> >>>
>> >> >> >> >>>Add a warning if you want to deprecate it later.
>> >> >> >> >>>
>> >> >> >> >>Isn't the time to make an interface change now rather than later?
>> >> >> >>
>> >> >> >> I would say that the time to make an interface change before
>> >> >> >> 1.0 has passed: I see more value in sticking to
>> >> >> >> to what we have claimed, than in fixing this single instance.
>> >> >> >>
>> >> >> >> >True, but last time we discussed this was 1 hour or so before the
>> >> >> >> >release of 1.0-rc1. Now we have a whole week to 1.0-rc2... :-)
>> >> >> >> >
>> >> >> >> >Marie, can you check again if that constructor is necessary?
>> >> >> >>
>> >> >> >> I'm typically using it for the same as the dolfin la/eigenvalue demo
>> >> >> >> is using it for.
>> >> >> >> Do you have a replacement syntax available?
>> >> >
>> >> > Marie, I think this should work:
>> >> >
>> >> >  u = Function(V)
>> >> >  u.vector()[:] = x
>> >> >
>> >> > where x is the solution you get from the eigenvalue problem.
>> >> >
>> >> > Can you see if that works?
>> >> >
>> >>
>> >> It won't,  because of a bad flaw in the vector assignment. It will
>> >> make u.vector()[:] a copy of x (which has the wrong parallel layout) ,
>> >> when what we want is to assign just the values.
>> >
>> > I thought you argued before that was the correct behavior? (When we
>> > discussed the subfunction assignment last week.)
>> >
>>
>> I argued that it is being done consistently, but not that it's right.
>> We need to distinguish between copying and object and assigning just
>> values of a vector (but not touching the layout, ghost values, etc) to
>> another vector of the same length.
>
> Agree.
>
>> > Is it much work to fix that before 1.0-rc2?
>> >
>>
>> Proper assignment is too much work. I'm adding a work-around fix now
>> so the eigenvalue demo should be correct in parallel.
>
> What are the implications for whether we should remove or keep that
> constructor. We should only remove it if we can provide a working
> alternative (assignment operator working).
>

We can remove the constructor and provide an alternative that is no
more broken that what we have now, but which has scope for an easy fix
later.

Garth


> --
> Anders
>
>
>> Garth
>>
>> >
>> >
>> >> Garth
>> >>
>> >> >
>> >> >
>> >> >
>> >> >> >> That said, I'm not going to lose any sleep over this.
>> >> >> >
>> >> >> > Is everyone ok with throwing an error that it doesn't work in
>> >> >> > parallel?
>> >> >> >
>> >> >>
>> >> >> I don't think that is ideal.
>> >> >>
>> >> >> I building now with the constructors commented out to see how many
>> >> >> changes would be required.
>> >> >
>> >> >
>> >
>