Added lorenz shaped profile for the hidden region - applicalbe to UV2

[donal-mur]

Here I have added a fill function with a bump to better fit the UV2 type profiles in the hidden region - Called it fill_lorentz

It assumes a given position for the max of the bump currently -58 binned rows (bin by 2) from the bottom of the image. This should be instrument pointing dependent - (needs checking )

The calibration function calls need to be adjusted to apply this the the UV2 data (@lindamegner) I also recommend that we use fill_exp for the other channels rather than fill_lin

[lindamegner]

So you mean that desmear_true_image for UV2 should be called with fill_method == "lorentz", but the others with what method?

[lindamegner]

Someone who has more large code thinking than me, a general question: Is it good to restructure the whole code like this to a better syntax as this push is doing? I mean of course it is better with at better syntax, but there must be some risk for errors; even if the actual syntax change is done by the computer, one could happen to type something, and that would be hard to spot when there are so large changes.

[donal-mur]

With fill_exp like I said

On 12 Mar 2024, at 06:29, Linda Megner @.***> wrote:

So you mean that desmear_true_image for UV2 should be called with fill_method == "lorentz", but the others with what method?

— Reply to this email directly, view it on GitHubhttps://github.com/innosat-mats/MATS-L1-processing/pull/220#issuecomment-1990569270, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AO72MFCRTWWUHP6VARG2L3TYX2HE3AVCNFSM6AAAAABEIYRF62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJQGU3DSMRXGA. You are receiving this because you authored the thread.Message ID: @.***>

[skymandr]

Someone who has more large code thinking than me, a general question: Is it good to restructure the whole code like this to a better syntax as this push is doing? I mean of course it is better with at better syntax, but there must be some risk for errors; even if the actual syntax change is done by the computer, one could happen to type something, and that would be hard to spot when there are so large changes.

A couple of pointers and personal oppinions:

• I am all for good coding style (which to me is not the same as syntax), but a project should agree on what coding style to use before enforcing it.
• Ideally, unit tests should be run on each PR, to ascertain that changes do not affect functionality unexpectedly, thereby giving developers the confidence to implement better code style. Github has support for this. [0]
• Ideally, agreed coding style should also be enforced automatically, so that PRs are not accepted if they don't follow the agreed coding style. black or flake8 are good options for this.
• However, syntactic and/or algorithmic changes should never be in the same commit as code style fixes, precisely to avoid the problem of spotting what is what when reviewing.
• Ideally major code style fixes should be in a separate PR.

Long story short: don't mix style and logic; agree on what style to use.

---

[0]: The Lambda part of this repository uses this, see: https://github.com/innosat-mats/MATS-L1-processing/actions/runs/8170337140/job/22336279776?pr=220 – unit tests, code style ("lint check" using flake8, and type checks using mypy are required to pass for this part of the code, or the PR is not allowed to be merged.

[lindamegner]

OK. So, the only thing that is not syntax is this push is the function desmear_true_image? If so, I suggest we follow what the wise Skymen suggest and either: a) redo the push without the syntax change b) send the desmear_true_image code to me and I can add it in teh same push as I change the call so that uv2 is called with fill_method == "lorentz" and all the other channels with fill_method ==fill_exp

[lindamegner]

Then we can think about if we should change to more proper the syntax or not.

[donal-mur]

Finally I think I understand what you are talking about - by syntax change. you mean that I let VSCODE reformat the file ? Sorry about that. - I guess it created the appearance of multiple changes. datasets = [ds for ds in [xr.open_zarr(s) for s in stores] if len(ds.data_vars)] if len(datasets) == 0:

I don't think that I can undo that now .

The bit you need to add is this

elif fill_method == "lorentz":
        x0 = -(
            int(nrskip / nrbin + 1) - 58
        )  # 58 may need to be adjusted as a function of the pointing
        y1 = np.median(image[2, :])
        y2 = np.median(
            image[4, :]
        )  # took row 5 index 4 to get a better handle on the gradient
        gamma = 1 * np.sqrt((y1 / y2 * (2 - x0) ** 2 - (4 - x0) ** 2) / (1 - y1 / y2))
        fillx = -(np.arange(nrskip / nrbin) + 1)[::-1] - 1
        fill_function = np.expand_dims(
            gamma * gamma / ((fillx - x0) ** 2 + gamma * gamma), axis=1
        )
        filtered_row = median_filter(image[0, :], size=11, mode="mirror")
        # filtered_row=image[0, :]
        A = np.expand_dims(
            filtered_row / gamma / gamma * ((0 - x0) ** 2 + gamma * gamma), axis=1
        )
        # print(len(filtered_row))
        fill_array = (A @ fill_function.T).T

So you can put it in your version of the code and do a new commit. We can just delete this branch

[lindamegner]

OK. Good. I will do this on Friday.

[lindamegner]

added in other pullrequest