Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2161 RID LLu Critical Algorithm 10.9 IFU image and cube reconstruction needs more work #273

Open
hugobuddel opened this issue Nov 1, 2023 · 10 comments

Comments

@hugobuddel
Copy link
Contributor

https://jira.eso.org/browse/MET-2161

Critical Algorithm 10.9 IFU image and cube reconstruction needs more work

As part of its FDR deliverables the consortium has delivered a framework with prototypes of four critical algorithms. While the overall framework and three prototypes could be reviewed, the "IFU image and cube reconstruction" prototype could not.
The necessary run-time environment for this prototype to run could not be established within a reasonable time frame. A review of the prototype source code suggests that some work is still needed.

@hugobuddel
Copy link
Contributor Author

Making an issue so we can discuss our answer.

Directly addressing the RIX would just be:

We will improve the IFU cube reconstruction critical algorithm to be easier to run and will update the instructions.

Perhaps we can elaborate on the algorithm itself.

The scope of the algorithm will also be clarified. The algorithm proofs that we can perform the cube reconstruction for specific cases which we anticipate we can generalize.

Something like that?

@sesquideus
Copy link
Collaborator

I think the main point is that we need to discuss this with PWb first and see if there is any reason why we should not use the matrix approach. If there are any compelling reasons not to use this or there is some unforeseen problem, we can always fall back to the MUSE approach. I would hate to discard what @oczoske has done and I like matrices somewhat better.

As for the algorithm the preparation part is complete. We can create matrix A in a few hundred ms for general rotation and in negligible time if model and data are aligned or rotated by 90°.

@hugobuddel
Copy link
Contributor Author

So what do we answer? We can't discuss anything with PWb first, because we need to submit tonight. I can't answer if I don't know what we plan to do, because @sesquideus is probably the person who will actually do it.

The actual RIX states that they could not run the Critical Algorithm, so the first thing we need to decide is whether we want to improve the documentation of the algorithm (or the algorithm itself) so Lars can run it. Do we plan to do that? Unless I hear otherwise, I'll assume yes.

The subsequent issue is the algorithm itself, do we already want to indicate that we are planning to improve on that too? I'll reply something vaguely.

Maybe:

We will improve the IFU cube reconstruction critical algorithm to be easier to run and will update the instructions.

We are also considering improving the algorithm itself and are in the process of discussing that.

@astronomyk
Copy link
Contributor

Copying in Lars' response to the question during the 2023-10-30 weekly telecon of "how to proceed with this RID so as to be able to move forward during the FDR meeting":

Hi Kieran,

As agreed today here is my follow-up on RID MET-2161

It is probably not constructive to attempt to demonstrate the prototype
at the FDR meeting.

Instead I propose:

  1. To the extent possible, we (the METIS DRLD team and I) iterate on
    the ticket prior to the meeting (to possibly reduce the scope of
    the RID).
  2. For the FDR meeting you prepare an answer (in text form) to explain
    why you are in fact able to resolve the RID.
  3. The actual FDR-meeting will then (hopefully) agree on an Action Item
    for you to resolve the RID, and then the FDR will be passed pending
    the successful
    closure of that Action Item.

It should be noted that so far no reviewer was able to review this
prototype.

As such, the Action Item should realistically consist of a new delivery
of the prototype, in a form where more than one reviewer can sign off
on it.

-Lars

@sesquideus
Copy link
Collaborator

We definitely have to provide a working prototype. I have already told Lars that right now it does not "run" -- in the sense that you cannot put in six synthetic "exposures" and obtain a single reconstructed image. I still need some time to get it to that state. I cannot promise that it will be done in a week though.

As for the algorithm, I have no reason to think what @oczoske has written would not reconstruct the image, except for the equation at the end it is clear to me now. The problem is that for the real size of the matrix (16800×4624) the inversion took forever to run with both numpy and Eigen (quite literally, it did not finish in eight hours). Maybe I am doing something completely wrong -- I still think it is a problem of the implementation, not the algorithm itself.

But if there are going to be any compelling reasons to use pixtables instead, I think much of the codebase will be common anyway. We will still need to determine overlaps and fill factors and determine which exposure contributes where, and this is already working. Only instead of assembling them into a matrix we process the pixels sequentially. But I would like to finish reading the MIRI paper and talk to Peter first.

Right now I doubt we can do any better than a vague answer, especially given what Kieran just posted here.

@astronomyk
Copy link
Contributor

Indeed given what Lars wrote, I don't think he expects us to show the full working prototype at the meeting. As @sesquideus says, there is not enough time for that.

I see the discussion of this RID at the meeting having three parts:

  1. Practically, why have we not been able to show a working prototype. E.g. Why do we not have an ipython notebook for example showing what we have achieved so far. And what is missing from the code that we have to be able to fully show proof of concept of this idea? Just as a reminder, the purpose of the crit-alg section is to show proof of concept, not to have a fully working implementation. But in this case, I guess proof of concept required a (more or less) complete implementation (?)

  2. What is special about this algorithm? Why have we decided to follow this path, rather than reuse what already exists in the HDRL (i.e. the MUSE pixel-tables solution). For this we --need-- to have a good answer. Otherwise, the panel will simply say we shouldn't fall into the trap of the sunk-cost fallacy, and then force us to use the MUSE HDRL functions. I think we should therefore spend next week attacking this point as best we can. I'm happy ask P-Wb for a meeting next week to discuss the question of why they didn't use the matrix approach. And iirc Jane Morrison said she is happy to answer questions on the MIRI reconstruction algorithm. I'll ask her for a videocon too. @sesquideus please also make sure to get a good understanding the cube reconstruction implementation from the Law+23 paper.

  3. What needs to happen to close this RID? I fear this will be the item that results in a "RIX Closed with Critical Action Item". According to Felix, "Critical AIs" (as opposed to major, minor, and trivial AIs) are the only ones that actually block us from passing FDR. If we want to avoid a critical AI we will need a plan forward. I suggest something along the lines of:

    • Martin spends a further month working on the implementation
    • We reconvene (half day telecon) with a subset of reviewers mid December to either show the working prototype, or discuss the problems associated with it. (Again working prototype can be on a smaller data set e.g. 6x 256x256)
    • If the reviewers agree the problems are surmountable, they can grant close the AI, or agree on a further review date no later than end of Jan 2024.
    • If they say that they are still unhappy with how it looks, we throw in the towel and go the MUSE route.

I'm open to any suggestions or change requests regarding this approach.

Regardless of the details though, we --will-- need to address these three questions in the slides we present on this RID.

@astronomyk
Copy link
Contributor

astronomyk commented Nov 1, 2023

Here's what I propose as an answer to the RID:

We accept (and regret) that the submitted code base was not sufficient to show the functionality of our proposed implementation of the IFU image and cube reconstruction algorithm. Unfortunately the FDR review will not be the correct setting for any additional in-depth code review with the relevant ESO reviewers.

To close this RID it will be necessary to convince the reviewers of the viability and implementability of the proposed reconstruction algorithm. To do this we propose the following course of action:

  1. A discussion during the FDR meeting on the reasons for pursuing our variation on the theme of cube reconstructions. Our aim here is to convince the reviewers that we are indeed on the right path.
  2. A new delivery date (TBD) for the updated cube reconstruction prototype code, as well as an additional (half-)day working meeting/videocon (date TBD) with the reviewers to discuss and demonstrate the new prototype code.
  3. Following the second code review, the reviewers shall decide to a) accept the prototype code, b) allow additional time for development and a third review, or c) abandon our proposed algorithm in favour of an existing solution (e.g. MUSE, MIRI, etc).

@sesquideus @hugobuddel what say you?

Edit: typos

@hugobuddel
Copy link
Contributor Author

Something like that is fine. Maybe it is a bit too much.

(Disclaimer, take the below with a grain of salt, I'm tired.)

E.g. we could perhaps remove the entire 3th bullet, because 3a and 3b are kinda implied, and w.r.t. 3c I don't want to give ESO control over which algorithm we use. As in, it is also implied that they can reject our prototype and escalate [e.g. not pass FDR] if we don't choose the other algorithm, but that is different from giving them the decision power. We could perhaps add a sentence (after the bullets) about that we also investigate the applicability of the MUSE/MIRI code as a backup solution.

Also the first sentence of the second paragraph is perhaps not necessary. It's not our job to tell the reviewers what they need to close the RIX. Perhaps just adapt the last sentence to "To close this RID, we propose the following course of action:".

And I would perhaps not use "convince" in the first bullet, but just "show". This works both ways:

  • We don't want to convince anyone, we know we're right, and it's not our problem if others aren't convinced. (Yes it would be our problem, but only because they make it so, not because we care.)
  • They will not be convinced during the meeting. People usually get convinced at a later point when they have time to digest the arguments presented.

Explicitly mentioning the dates, even though they are TBD, is great, because this keeps us in the drivers seat. We don't want them to feel forced to impose meetings upon us to supervise us; instead we take the lead by inviting them.

Blablabla, I wouldn't be able to implement this algorithm, or make it even more complex, so I'll let you decide how to answer.

@sesquideus
Copy link
Collaborator

I hope that for the next two or three weeks there will not be too much other stuff on my plate anyway, so I can get to work on it full time. Providing a mini-version should be possible too. The third bullet seems a bit pushy to me too.

@astronomyk
Copy link
Contributor

Points taken. Here's an updated version:

We accept (and regret) that the submitted code base was not sufficient to show the functionality of our proposed implementation of the IFU image and cube reconstruction algorithm. Unfortunately the FDR review will not be the correct setting for any additional in-depth code review with the relevant ESO reviewers.

To close this RID we propose the following course of action:

  1. A discussion during the FDR meeting on the reasons for pursuing our variation on the theme of cube reconstructions. Our aim here is to show the reviewers that we are indeed on the right path.
  2. A new delivery date (TBD) for the updated cube reconstruction prototype code, as well as an additional (half-)day working meeting/videocon (date TBD) with the reviewers to discuss and demonstrate the new prototype code, and to discuss further actions

@astronomyk astronomyk changed the title RID 2161 LLu Critical Algorithm 10.9 IFU image and cube reconstruction needs more work 2161 RID LLu Critical Algorithm 10.9 IFU image and cube reconstruction needs more work Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants