Dalework

[githubdoe]

Updated percent correction feature.

[gr5]

You didn't pull from master so there was a small conflict. I fixed the conflict in dalework but now the repository version of dalework doesn't match yours. Please grab dalework asap otherwise you might start to do more edits to dalework and then there will be more conflicts:

git pull

Also after this pull request is approved and merged, there is a very small edit that you didn't get. So after this PR is approved do:

git checkout master
git pull (this updates your copy of master)
git checkout dalework
git merge master (merges in the tiny edit)

[gr5]

I built and tested DFTF from dalework but something seems wrong. The "show correction" graph looks okay and I tried saving the zones and I noticed the ROC value in the file was zero.

I looked quickly over the code and I don't see how this is possible. It has the correct value in the mirror dialog. The code to save the json file looks fine.

What are your thoughts? I can look again later.

[gr5]

Okay I looked at the code more carefully.

So you are only intending to save the zones. The other values weren't intentional and are ignored when you read the zone file back in. So it's not a bug.

Or is it...

So you use registry or "QSettings" to save and load stuff from the registry.

1. This is a little scary to me as what happens when one wants to do a different mirror - does it pull in the zones from the previous mirror which might not make sense?

2. At the moment when you save the zones to the registry, m_roc is zero. Why would it ever be zero? I don't get that part. Is this indicative of another bug?

3. If anyone looks at the zone file they will see a bad ROC value of zero. They might think this is a bug. Or they might try to edit it expecting some change in behavior when the load the zones back in.

When looking at this I found it helpful to have regedit open and paid attention to "correctionZones" key in the registry as I was messing around.

On another note, it is jarring to me every time I see "knive". Knives is plural of Knife. e.g. 1 knife, 2 knives. There's no such word as "knive".

[atsju]

My mandatory annoying comment about fixing new warnings :)
Maybe I should automate a check that will fail on warnings. What do you think ?

[githubdoe]

Perhaps I have a local branch called master which is messing with us when I do your commands. Here is a screen shot of git bash that I use. I typed git branch and I think it lists the branches on my local machine. What do you think?

[githubdoe]

There is/was code that checks to see if the mirror parameters have changed since the percent parameters were last used. If that is the case then it does things to the necessary percent parameters.

Perhaps at one time I thought it would save the ROC in the percent parameters but then never really needed it but never removed the saving of it always using the value from mirror config. Maybe I wanted to save the ROC to watch for mirror config changes. Maybe I never realized it was always 0 and thus always caused the percent parameters to update themselves. Maybe.....

I will review when I get time.

[atsju]

Hi Dale.

I like to use gitk --all to display the commits and branches. It's graphical and interactive. I do it basically every time I commit.

The reason why you master was up to date but not up to date when you pull: you need to understand git has your local copy and the server copy which it calls "origin". When you checkout master, your local git does not know that origin/master branch has changed. Thus your local master branch is up to date with your local copy of origin/master branch. You can just do a git fetch every now and then. This will update the local copies of everything. Not dangerous command at all. Then you will see the origin/master branch move. And master will be shown as not up to date compared to your local origin/master.

[githubdoe]

Yes I try to remember to check warnings but my old habit is I ignore them during debug and then forget to look at them since they usually are not an issue and just busy work.

[githubdoe]

Thanks for the explanation on master. So the issue was I always stopped at master not being out of date. I do have the GitK gui. I got overwhelmed by it's display and never tried to learn what it all meant. On my screen what I think is the most interesting part at the top is a small window that I can't figure out how to enlarge that shows just a few lines.

Update on percent correction and different mirror than last used. At one time that feature would re-compute the zernike smoothed profile which could be time consuming. I wanted to keep that from happening when not necessary. So I tried to check if it was necessary. In the end I did all that a different way and the mirror parameters will always be up to date because the calling function (profile plot) will give it the correct parameters when it wants a plot. The "generate button" on the percent GUI tells profile plot to request a plot so that it can give the right mirror parameters.

So there probably is some old code laying around that is not used and indeed one of the warnings is highlighting that I think.

[githubdoe]

I intended to also save and reload the exclusion radius from the zone file. But don't remember if I completed that. Will check.

[githubdoe]

Just wanted to add that yes warnings are useful. It is just many years ago when they first started to appear from C compilers many were not relevant and it created a lot of wasted time going through them all. So most people ignored them. That is the habit I got into.

[gr5]

Here is gitk -all

It's not obvious but if you mouse over where I put the dashed red line, the cursor changes and you can click and drag to make the top half larger. I think it's worth understanding this tool as it will help you understand branches better.

Julien mentioned "git fetch" which I don't do often but have many times in the past. It grabs all the updated branches from github to your machine so you can see both your own branches and the "tracking branches" which match what's on github. "git pull" does a fetch for only your current branch name and also moves your current branch to the latest commit. "git fetch" doesn't change anything you might be working on - it just updates your repository's tracking branches. Not the working branches. So it's super safe. Sometimes I do it before a git pull because then I can see if anyone has updated some branch before I do a pull or merge.

[gr5]

I don't know what you are talking about regarding percentage. Here is what is in the zones file:

{"ROC":0,"mirror radius":100,"zones":[0.08,0.3,0.51,0.66,0.77,0.87]}

All of that info is also stored in the registry under "correctionZones". But when reading the zones file you only read in the zones array. So I thought maybe get rid of the other stuff from both regedit and also zones file?

Also I'm concerned that zones won't get reset if someone loads a new mirror. But I'm not sure what will happen. What is the reasoning for storing the zones in the registry?

Or maybe there should be a button to automatically setup default zones based on the quantity of zones?

I'm just confused about how people are going to get default zones if DFTF always has old zones in the registry and it uses those.

[githubdoe]

"Percentage" is my short cut way of talking about the "percentage correction" feature, the one we are currently discussing.

The registry saves the last settings of this feature. One does not need to save those setting in a file if they do not want to. That probably is the usual case. They can change them and the registry will always have the last values. However maybe they want to recall settings they used sometime ago on a different mirror that they saved in a file. They can do that provided they saved them. Which is what the load and save is all about. Just like the load and save of the mirror config. It too remembers your last used values even if you don't save them into a file.

The current mirror value has the values of the mirror that must be used by the percent correction feature so I decided to let the profile plot feature always give those to the percent correction feature when it wants a percent correction graph. That way the current mirror is always what is used. Those parameters in the zone file are moot now and not used. I will look at removing them in the save.

[githubdoe]

Oh, yes now I remember saving those in the zone file and the registry gives me a way to know if the mirror parameters are still the same as before. If not the feature should ask if you need new zones. That last part never got reimplemented.

I planed on asking if they wanted to update the zones or use the old values. Similar to what I do when loading a wave front with different config values.

[githubdoe]

There was a race condition that allowed the user to save zones before knowing the ROC. I don't think that can happen now. So George perhaps your zone file was created too long ago. I think recent version do not have that issue. At least my test of it saved the current mirrors parameters. As I said earlier I will use them to notify the user that the mirror has changed and see if they want to update the last zones used.

[gr5]

Okay I under stand what you are saying now and I now understand what you were saying before as well. Let me know when you've pushed the changes to dalework (don't forget to make your edits in dalework instead of in master).

Tomorrow I have more time than usual to look at your changes.

I think the ROC got set to 0 in an older version in the registry and the changes persisted. So anyone who has the older version 7.3.3 will probably have an ROC set to 0 unless your new code notices it's a new mirror and prompts the user.

[githubdoe]

Ok, Ok, Now I remember I was thinking much better when I wrote the original code. It stores zones as a percentage of the mirror's diameter. Duh. So no need to worry when the diameter of the mirror changes. The zones will be created from the load in the same ratio as they were on the other mirror.

[gr5]

So is it done? No changes needed? Should we go ahead and pull this?

[githubdoe]

It is not done but closer than we thought.

[githubdoe]

Git has given me info that I don't understand. It says I should pull but when I try it says I have unmerged files and tells me to fix them in work tree. What ever that means.

[githubdoe]

Here is the git status which has a problem with revisionHistory.html. Why the problem? Why does it say "Both modified" ? I thought I had done a merge with the remote and so expected it to merge. It had a merge conflict and I manually edited that conflict (I thought). I saved that edit I think. Then this error.

[githubdoe]

I don't understand why what I did before to do a manual merge did not work. It did not work the second time either but has on the 3rd try.
So I pushed my changes.

[gr5]

After you fixed the merge conflict maybe you have to create a commit? I don't remember.

[gr5]

I'm telling you again:
gitk --all

It makes everything more understandable.

[gr5]

I can see that master has been merged into dalework so I think you are in good shape and can make more edits now as needed.

I think the most recent merge conflict maybe had to do with blank lines being different in master versus dalework. I'm not certain.

[githubdoe]

I tried gitK and got it to enlarge the display. Yet it is just a graph of stuff with little meaning to me. sorry. It sure did not help solve the issue. The first problem was my lack of understanding of what an "add" does. I thought it added a file to the local git archive. So I was puzzled by why it recommended me to do an add of that file. It was already in the archive. Eventually I gave up and did the add. Then went and looked what an "add" does. I don't use it directly so that is why I did not know about it. I always use a commit -am. I have never really understood what staging does. Seems like a commit would be enough. Oh well that's git.

So really about all I had to do was add then merge, manual reconcile, then add then commit. Or something similar that eventually cleared up the problem.

I'm done with changes and ready for more testing by others.

[gr5]

Right. Add and then commit. Well it's easy to just wildly do a "git add *" But then when you do a git status you see you've staged lots of files you don't want to commit (possibly). So personally I like that it's two steps as you can easily remove files you added accidentally. Then I do the commit.

I usually have a few files hanging around that I don't want to be in the repository.

I think it's really worth it to understand gitk -all

If you don't understand the rows in the upper window then you are missing some understanding of rows and branches. And seeing what is bold is very helpful as it shows you what commit you currently have in your working folder. It shows if you have files added but not committed or worked on but not added.

And it shows you how everything is merged - what was not merged is most important. You can quickly see that "oh - I thought I merged master into dalework but I can see now that the version of master on github didn't get merged in". Or whatever it is that I may have messed up personally.

When git says you can't push or can't pull, I find the doing gitk -all helps explain the issue. And I can click on each row and see what exactly the changes were (often just one line of code or even just one character) and see how it's no big deal to just merge everything into my current work branch.

I used to do gitk -all and then after every git command I would go back to that window and refresh it to see what my git command just did.

There are several other free gui's created by other people that show a display similar to "gitk -all". You could install one of them. By the way, have you noticed when you are editing a file in QT it sometimes shows the person, and the date of when that line of code was edited? It's getting that from git. In gray to the right of the line of code.

I'm traveling today and "celebrating father's day" but I may have time this morning or much later today.

[gr5]

When you say it's all set you mean the update you did last night at 6:15 to 6:19pm, right? Lots of changes committed then. Nothing since.

[githubdoe]

Yes,