Remove deprecated routines

[d7919]

Removes:

• DataIterator is deprecated in favour of the new Region and Ind2D/3D/Perp family. This should not affect user code -- if it does, replacing DataIterator with auto should do the right thing in most cases
• DataFile::writeVar: use DataFile::addOnce
• Field::setName and Field::getName: just use Field::name directly instead
• Field::error and bout_error: use BoutException instead
• rvector/rmatrix/rtensor families of functions: use Matrix/Tensor instead
• operator^(Vector2D/Vector3D): use cross() instead
• The derivative function overloads with this order of arguments: DD?(..., DIFF_METHOD, CELL_LOC, REGION). Instead, use DD?(..., CELL_LOC, DIFF_METHOD, REGION)
• Vector derivative function overloads with three separate outloc_[xyz] arguments: use the versions with a single outloc argument instead
• CyclicReduce::setCoefs and solve overloads that take T[] or T**: use the version that takes Array instead
• The FCI class constructors that take a bool yperiodic argument: this is no longer supported
• Mesh::coordinates is deprecated in favour of the more consistently-named Mesh::getCoordinates. There is also now Field::getCoordinates which may be more convenient

[d7919]

Note there are a few DDZ overloads in derivs.hxx that are noted as being deprecated but didn't have the deprecated property tagged to them. Should we remove these now as well?

There are three boundary choices that are reported to the user as being deprecated ("dirichlet_2ndorder", "neumann2" and "neumann_2ndorder") can we remove these as well?

[ZedThree]

I'm not sure why we didn't just mark those DDZ overloads as DEPRECATED. I think they can be removed.

I think those boundary choices have been deprecated for a long time now, should be ok to remove them.

[d7919]

Should I remove those other items here or in a separate PR?

[ZedThree]

Do those in a separate PR -- all this can definitely go, I'm less sure about those ones.

[d7919]

There are two fourth order dirichlet and neumann boundary types, I'm guessing at least one of each should be removed, but I'm not sure which.

[johnomotani]

I'm attempting some tidying up of boundary conditions at the moment (I got angry with boundary_standard.cxx when trying to fix minor bugs in it last week). I'm almost by default cleaning up the dirichlet/neumann options as part of that. So maybe you can leave this to me for a little bit and revisit if it turns out my re-write is not as good an idea as I hope....

[d7919]

@johnomotani sorry I didn't see this until after I'd made #1331. Happy to remove the boundary removals from there if it's going to cause you headaches.

[dschwoerer]

Indices was also removed.
What can I use instead?

[dschwoerer]

Also removing something after being deprecated for 17 days feels pointless to me. Not everybody pulls that often - thus it is easier to directly remove it.

[ZedThree]

Indices was also removed.
What can I use instead?

Ind2D/Ind3D/IndPerp, or even better auto -- DataIterator and Indices probably shouldn't appear directly in user code.
Note that x, y, z on these are now methods rather than members.

We've deprecated them in master which is the branch you should probably be using for production or publication (more stable, easier to reference in publication). Most of these things were deprecated since the last release.

[d7919]

Depending on what you're trying to do you may prefer to start by providing a local definition of Indices (struct Indices {int x, y, z;};) with consideration of the use of alternatives deferred to the code review stage?

[dschwoerer]

I replaced DataIterator with auto - which fails as auto is a std::initializer_list<int>
This completely brakes the aiolosmesh, and also the nonuniform boundary conditions.

SpecificInd seems to not support arbitrary offseting - which means a LOT of extra handling in using it, compared to DataIterator. Is the slight optimiaztion this allows really worth all the extra checking in whereever I use it? Also requiring CHECK=3 for detecting bugs that are introduced that way, seems slightly awkward.

[dschwoerer]

Also DataIterator had a modulo free zm/zp version, which should be much faster.
Is it worth to copy that over?

[d7919]

It should support arbitrary offsetting (SpecificInd::offset takes an offset in each of the three directions) unless I misunderstand what you mean by arbitrary offsetting.

[dschwoerer]

/// Assumes that the offset is less than the grid size in that                                                                                                          
/// direction. This assumption is checked for at CHECK=3. This                                                                                                          
/// assumption implies that a `FieldPerp` cannot be offset in y, and a                                                                                                  
/// `Field2D` cannot be offset in z. A stronger, more expensive check                                                                                                   
/// that the resulting offset index doesn't go out of bounds can be                                                                                                     
/// enabled at CHECK=4.     

[ZedThree]

I replaced DataIterator with auto - which fails as auto is a std::initializer_list<int>

Can you show the code? If you've done the following: auto i = {1, 2, 3}, yes, that's a std::initializer_list<int>, in which case you do need to name the type.

SpecificInd seems to not support arbitrary offseting

const inline SpecificInd offset(int dx, int dy, int dz) const does the job?

Also DataIterator had a modulo free zm/zp version, which should be much faster.
Is it worth to copy that over?

Unfortunately, that version doesn't work unless we keep track of x, y, z and make sure they are in-sync with the single index, which either destroys the benefits of SpecificInd or requires a modulo anyway. We are planning on implementing periodicity in Z via guard cells in the near future, which will have many benefits, including removing the need for the modulo here (as the index won't wrap around).

[d7919]

If you want to offset by an entirely arbitrary amount you can add any integer to the public ind member using the defined operator+ methods.

[ZedThree]

What are you doing that you need such large offsets? We didn't have any use cases that needed them, which is why we didn't consider it.

[dschwoerer]

I would like to be able to call xp(1) and get an error if there is only one point in x. I guess that happens if CHECKS>2 - which is fine. I would also like to be able to call zp(2) and get the identity if z=1 - which might work.

The issue is - even in a Field3D any offset, larger than zero can be equal or larger then the number of points in that direction.
These large offset are e.g. used in derivatives or any other non-local code.

Honestly, at this point I am mostly confused as to what is meant by that comment in the code.
Offsets in non-periodic direction are anyway not valid, thus only checking with appropriate error seems only natural to me. The only thing that might be broken is the z direction - if the offset is more then 2*LocalNz

Anyway, SpecificInd seems fine, sorry for the confusion. I will try to find and understand the constructor for the region.

The code is here: #742 and #1179

[d7919]

I think the motivation for not having checks directly within xp is that in depending on your viewpoint it's not an error to have an index referring to a point outside the defined grid, it's only an error when you try to use this to index the grid. The checks that appear in yp could be ported to xp to prevent this usage of the ind.

[dschwoerer]

No, I think I am fine with the implementation, it is more so that the documentation has severely confused me.
Especially as it is not true for Z - which is probably the only one where that behaviour is well defined if the index is used, handles that case correctly. Should that warning be removed?

[d7919]

If the documentation is confusing we should definitely look to improve this.