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

Refact: StreamTask, OATask - order of processing/adaption on new/obsolete instances #988

Closed
27 of 35 tasks
detlefarend opened this issue May 19, 2024 · 6 comments · Fixed by #990, #992 or #1006
Closed
27 of 35 tasks
Assignees
Labels
BF Basic Functions/Infrastructure OA Online adaptivitiy quality Quality assurance refactoring Restructuring, renaming, simplification, understandability, ...

Comments

@detlefarend
Copy link
Member

detlefarend commented May 19, 2024

Motivation
Currently, methods StreamTask.run() and OATask.adapt() get instances to be processed in two different parameters

  • p_inst_new : list[Instance]
  • p_inst_del : list[Instance]

As a result, both lists are processed separately and block-wise.

Since an instance has a unique time stamp (see issue #987) the processing/adaptation should take place in an overall order by time stamp.

One possibility is to replace parameters p_inst_new, p_inst_del by a new parameter

  • p_inst : dict[tstamp,tuple(new_del_flag, Instance)]...

Benefit
Processing/adaptation is much more plausible than before.

Are the proposed changes backward compatible?

  • Yes
  • No

Related development objects

  • bf.streams
    • bf.streams.StreamTask
    • bf.streams.StreamWorkflow
    • bf.streams.StreamScenario
    • bf.stream.tasks.Window
      • 2D plot
      • 3D plot
      • nD plot
    • bf.streams.tasks.Rearranger
    • bf.streams.tasks.Deriver
  • oa.streams
    • oa.streams.OATask
    • oa.streams.tasks.BoundaryDetector
      • nD Plot
    • oa.streams.tasks.Normalizer (Laxmikant)
      • NormalizerMinMax
        • 2D plot
          Plot raw/normalized should be identical (except scaling)...
        • 3D plot
          Plot raw/normalized should be identical (except scaling)...
        • nD Plot
          Plot raw/normalized should be identical (except scaling)...
      • NormalizerZTrans
        • 2D plot
        • 3D plot
        • nD Plot
    • oa.streams.tasks.ClusterAnalyzer
    • oa.streams.tasks.AnomalyDetector (Syam)
  • oa.systems (Laxmikant)
    • Refactoring of all methods using p_inst_new, p_inst_del (needs to be replaced by p_inst : InstDict). The best is to search for p_inst_new, p_inst_del in the code. Here are some findings:
      • Method OAFctBroken._run_wf_broken()
      • Method OAFctSTrans.simulate_reaction(): l. 213
      • Method OAFctSTrans._run_wf_strans()
      • ...
  • MLPro-Int-River
  • MLPro-Int-scikit-learn
@detlefarend detlefarend added quality Quality assurance refactoring Restructuring, renaming, simplification, understandability, ... labels May 19, 2024
@detlefarend detlefarend self-assigned this May 19, 2024
@detlefarend detlefarend added OA Online adaptivitiy BF Basic Functions/Infrastructure next release labels May 19, 2024
@detlefarend
Copy link
Member Author

@steveyuwono @syamrajsatheesh this heavy design weakness came to my notice during working on SPARCCStream. Unfortunately, the resulting changes are extensive. But better now than after releasing MLPro 2... What is your opinion? Do you see a better way to solve this?

@steveyuwono
Copy link
Collaborator

@steveyuwono @syamrajsatheesh this heavy design weakness came to my notice during working on SPARCCStream. Unfortunately, the resulting changes are extensive. But better now than after releasing MLPro 2... What is your opinion? Do you see a better way to solve this?

Hi @detlefarend , what do you mean by block-wise? Regarding the order of processing the instances, I agree with the idea of ordering based on its timestamp rather than first the addition and then the deletion.

Regarding the proposed solution, a dict is a good solution as the new parameter.

@detlefarend
Copy link
Member Author

Hi @steveyuwono, with block-weise I mean that we currently process all new instances in method _adapt() before we process all obsolete instances in method _adapt_reverse().

Ok, I will do the refactoring tomorrow in a separate branch. The best is to merge everything to main before. Any doubts, better ideas, last words?

@steveyuwono
Copy link
Collaborator

Okay, sounds good! Nothing else from my side...

@detlefarend detlefarend reopened this May 22, 2024
@detlefarend
Copy link
Member Author

@laxmikantbaheti I already refactored the normalizers due to the new stream instance management (see oa.streams.tasks.OATask, run() and adapt()). I also added numerous further howtos to validate every single oa task in 2d/3d/nd mode. Two things are not yet solved:

  • Normalizer MinMax: parameters are updated during plotting but there are still differences between input signal and scaled plot. I expected an identical plot in both windows. But this is maybe the wrong approach. The normalizer re-normalizes no internal data. Why should it do for the plot data? Let's talk about this.
  • Z-Transformation: not yet running at all. Equations need to be reviewed as mentioned in Bug: Normalizer Z-Trans needs to be reviewed #993

@detlefarend
Copy link
Member Author

@laxmikantbaheti I also started refactoring oa.systems. Here further things are to do. I added a brief description to the issue.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BF Basic Functions/Infrastructure OA Online adaptivitiy quality Quality assurance refactoring Restructuring, renaming, simplification, understandability, ...
Projects
None yet
4 participants