Change Cell to 'Center' and Face to 'Interface' to specify Field locations?

[glwagner]

No description provided.

[ali-ramadhan]

I think Center is much clearer than Cell but I prefer Face over Interface for brevity.

[tomchor]

Came across this by accident but I gotta say the Cell nomenclature really confused me in the beginning. This should be easy to change programatically though, right?

[glwagner]

I wonder if something like Tracer, Velocity rather than Cell, Face or Center, Face would be even better.

The reason is that the formulation really staggers 4 finite volume grids --- one for tracers, pressure, and mass, and 3 others for the 3 momentum variables. So cell-averages of all the quantities are located at "cell centers" of their respective grids.

Sadly that'd be a deeper change, since I think we would need to use something like xU, yV and zW rather than xF, etc.

Changing Cell to Center is just a lot of copy/paste. It's not intrinsically difficult. This issue should probably be resolved before a 1.0 (#1234)

[francispoulin]

This is an interesting issue and I don't pretend to have an answer but will add in my two cents.

We refer to the grid positions based on the fields, Tracer, Velocity, etc, because of the particular grid we are using. If someone ever decided they wanted to use a B-grid (not to say I recommend this), then things would change. I would think that picking names based on the geometry is more universal. But I don't think this would change the functionality at all, and half the people would probably find it confusing.

[glwagner]

That's a fair point. I guess we just have to remember that Center and Face refer to the tracer / pressure / mass grid, not the momentum finite volume grids.

[tomchor]

Just as a reference, the sed command to do this (I believe) is

sed "s/\<Cell\>/Center/g"

which produces the following results (as an example):

$ line="(Cell, Cellphone), Cell; :Cell, Cell.something"
$ echo "$line" | sed "s/\<Cell\>/Center/g"
(Center, Cellphone), Center; :Center, Center.something

So a one-liner to do this is (I think; I haven't tested):

find . -type f -name "*.jl" -print0 | xargs -0 sed "s/\<Cell\>/Center/g"

assuming we just wanna replace in "*.jl" files.

I agree with Center and Face being the most intuitive and I'd be glad to do this change if everyone agrees. Although this would probably need to come with a bump to the next version since this is a breaking change, right?

[glwagner]

Probably better to do this sooner rather than later if we really are going to do it. I think we're just dragging our feet because it will have widespread effects across a huge number of scripts that we've written.

@ali-ramadhan what do you think? Time to bite the bullet?

[johncmarshall54]

Greg, is this really neccessary? Other priorities? John

…

On Fri, Jan 22, 2021, 6:52 AM Gregory L. Wagner ***@***.***> wrote: Probably better to do this sooner rather than later if we really are going to do it. I think we're just dragging our feet because it will have widespread effects across a huge number of scripts that we've written. @ali-ramadhan <https://github.com/ali-ramadhan> what do you think? Time to bite the bullet? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#414 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKXUEQXN3PYIBNMGSS4LMT3S3FRHJANCNFSM4IXUGMDA> .

[tomchor]

@johncmarshall54 Fortunately it doesn't take any effort at all, since it's just one command. I did it on a local branch and tested it here, so it's pretty much done.

I think we're just dragging our feet because it will have widespread effects across a huge number of scripts that we've written.

Yeah, that's a bit annoying, but I'll share the command I used to do it here so that people can also use it on their scripts:

find . -type f -name "*.jl" -exec sed -i "s/\<Cell\>/Center/g" {} \;

[ali-ramadhan]

I'd be on board with the Cell -> Center change. Probably mostly a tedious change but sed to the rescue!

I'm not bothered by having to modify my scripts. If we agree on the change we should do it sooner rather than later.

I did it on a local branch and tested it here, so it's pretty much done.

If tests pass then you should open a PR! 🎉

[tomchor]

@ali-ramadhan All the tests passed!, but I think I screwed up when creating the branch and when I create a PR it also includes the commits I used for the KernelComputedField. I'm not sure what to do now...

Created a (I think) successful PR :)

[tomchor]

Apparently all the tests in my PR are passing now. Here's a question: I left CellField alone in this PR. Should I also rename it to CenterField for consistency?

[glwagner]

Should I also rename it to CenterField for consistency?

Yes.