Lv2 reads master #363

kakwok · 2018-02-28T13:43:55Z

Implements #285.
Partition-specific FM parameters can be set by adding an attribute like Partition=Laser to the element in the xml.
Instead of having LV1 reads MS and passes the results to the LV2s, LV2s will read MS and pick up the attribute specific to its partition if there are any.
The partition specific values (if any) will naturally override the settings directing to all partitions.
The partition specific values can live in both commonMS and main MS. Same MS-overrides-commonMS rule applies to them.
Example usage for HO's resync Bgo will look like this:

<ICIControlMulti>
 <include file="/TCDS/BGo_global.cfg" version="pro"/>
 <include file="/TCDS/global.cfg" version="pro"/>
 <include file="/TCDS/brildaq.cfg" version="pro"/>
 <!-- <include file="/TCDS/BGo_PedOnly_CalibSequence.cfg" version="pro"/>-->
 <!-- <include file="/TCDS/BGo_LaserOnly_CalibSequence.cfg" version="pro"/> -->
 <include file="/TCDS/BGo_CalibSequence.cfg" version="pro"/> 
</ICIControlMulti>
<ICIControlMulti Partition="HO">
 <include file="/TCDS/BGo_HO_resync.cfg" version="pro"/>
 <include file="/TCDS/BGo_global.cfg" version="pro"/>
 <include file="/TCDS/global.cfg" version="pro"/>
 <include file="/TCDS/BGo_PedOnly_CalibSequence.cfg" version="pro"/> 
 <!-- <include file="/TCDS/BGo_LaserOnly_CalibSequence.cfg" version="pro"/>-->
 <!-- <include file="/TCDS/BGo_CalibSequence.cfg" version="pro"/> -->
</ICIControlMulti>

This change is back-ward compatible to current mastersnippets.
RunkeyCfg (aka CfgToAppend) is also parsed by LV2, although cannot be made partition-specific as is.

kakwok · 2018-02-28T13:51:34Z

src/rcms/fm/app/level1/HCALlevelOneEventHandler.java

        functionManager.getHCALparameterSet().put(new FunctionManagerParameter<StringT>("GLOBAL_CONF_KEY",new StringT(GlobalConfKey)));

-        // RUN_CONFIG_SELECTED = LocalRunKey; CFGSNIPPET_KEY_SELECTED = MasterSnippet file of LocalRunKey
+        // CFGSNIPPET_KEY_SELECTED = LocalRunKey;  RUN_CONFIG_SELECTED= MasterSnippet file of LocalRunKey


The previous comment was wrong. Fixing this in the code to avoid further confusion.

kakwok · 2018-02-28T14:04:10Z

src/rcms/fm/app/level1/HCALlevelTwoEventHandler.java

-          }
-          else {
-            logger.debug("[HCAL LVL2 " + functionManager.FMname + "] Did not receive a request to perform special actions due to central CMS clock source change during the configureAction().\nThe ClockChange is: " + ClockChanged);
+            logger.warn("[HCAL LVL2 " + functionManager.FMname + "] Did receive a request to perform special actions due to central CMS clock source change during the configureAction().");


I don't see ClockChanged being used anywhere in the code. Keeping this warning message only in case we get a CLOCK_CHANGED from LV0

kakwok · 2018-02-28T14:09:44Z

src/rcms/fm/app/level1/HCALlevelTwoEventHandler.java

+      if(!CommonMasterSnippetFile.equals("")){    
+          //parse and set HCAL parameters from CommonMasterSnippet
+          logger.info("[HCAL LVL2 "+ functionManager.FMname +"] Going to parse CommonMasterSnippet(partition specific) : "+ CommonMasterSnippetFile);
+          xmlHandler.parseMasterSnippet(CommonMasterSnippetFile,CfgCVSBasePath,functionManager.FMpartition);


Parse the commonMS and MS for the 2nd time, looking for only partition-specific elements

jhakala · 2018-03-01T17:06:14Z

Hi @kakwok, in principle, would it be possible for the level1 to chop up the partition-specific info and send the level2s their specific command parameters? If not then I understand why the level2s all need to do their own parsing, but if the level1 could do that, then why did you choose this instead of that method?

kakwok · 2018-03-01T17:41:05Z

That will be tedious to implement because LV1 will have to conditionally attach different parameters into different commands and set the correct commands (with the parameters) to the right partitions.
We'll also have to make sure the partition-specific takes priority while playing this LV1-LV2 talking game.
It's conceptually much simpler for the LV2s to read their own.

jhakala · 2018-03-01T17:47:33Z

OK, that makes sense. The only comment I would make is that perhaps the code could be improved by refactoring such that the code added at line 522 of the levelTwoEventHandler does not duplicate the code at line 756 of the levelOneEventhandler. For example maybe that code could be moved into a method of the EventHandler and one could just use a one-line call to that method in both the level1 and level2 EventHandlers. This might eliminate needing to modify both spots if further improvements are made to what happens there.

kakwok · 2018-03-01T18:33:32Z

Fair point. CommonMS parsing could be absorbed into the parseMastersnippet function, like David did in the general parser, because they have to be parsed together anyway.

kakwok · 2018-03-01T18:38:45Z

Tested at P5.

No changes to all mastersnippets - local runs + minidaq run can configure.
Add partition specific tag in minidaq MS:
http://hcalmon.cms/cgi-bin/cvsweb.cgi/HcalCfg/Master/minidaq_HCAL.xml.diff?r1=1.14;r2=1.13;f=h
Picked up correctly in the parsing:

      INFO [HCAL HCAL_HO ] parseMasterSnippet: parsing TagName = ICIControlMulti with partition= HO
 2018-03-01 19:22:49 and 399 ms : cms.hcalpro.rcms.fm.app.level1.HCALFunctionManager
      INFO [Martin log HCAL HCAL_HO]: Going to read the file of this node from /nfshome0/hcalcfg/cvs/RevHistory/TCDS/BGo_HO_resync.cfg/pro

Logs look normal during test.

kakwok · 2018-03-02T12:10:17Z

We may have the case that more than 1 partitions share the same need for specific settings.
Relaxing the name check will reduce the duplication in snippets, e.g.
<ICIControlMulti Partition="HO;Laser"> will suffice.
The checking condition is simply HO;Laser contains FMpartition, where FMpartition is the usual FM name substring HCAL_XXX

…nes)

jhakala · 2018-03-06T16:38:48Z

Hi @kakwok, I would suggest changing this line because at certain teststands, the convention for FM names is different from HCAL_blahblah, for example at FNAL we name our l2 FMs HCALFM_blahblah. I think H2 also does:
https://gitlab.cern.ch/cmshcos/hcalConfMagik/blob/master/confmagik/hcal_H2_configs/function_managers.json
I suggest checking all the function_managers.json files we have in confmagik to verify the naming convention. I think the convention that they all have an underscore in the FM name is universal; so maybe change

FMqr.getName().substring(5)

to something like

FMqr.getName().split("_")[1]

jhakala · 2018-03-06T16:48:47Z

PS: alternatively, you could just use the full FM name instead of a shortened version, since other things, e.g. the maskedFM xml parameter, use the full FM name instead of a shortened version.

kakwok · 2018-03-06T17:00:07Z

I will allow both full names and partitions then. The partition convention is only true for P5 FMs. This has been the case since the time we developed FEDmasking

kakwok · 2018-03-06T17:11:01Z

Also implemented CfgScript veto.

kakwok added 10 commits February 26, 2018 14:13

Expand CheckAndSetParameter to setTarget parameter

f4f074e

replace more code with function

a284c34

LV2 reads mastersnippets

e739a06

Update RunkeyCfg log info with runkey name

1230a78

Send and recive runkey map

93f729b

receive MapT. Catch and handle exception

eaf2200

LV1 do not send MS parameters, LV2 do not receive MS parameters

a59b609

Parse partition specific parameters

fbf071f

Parse common and main for parition specific parameters

49249d6

Better way to pick-out partition specific elements

48fe097

kakwok commented Feb 28, 2018

View reviewed changes

Fix typo

29fbf07

kakwok added 3 commits March 1, 2018 20:54

Condense commonMS parsing and MS parsing into one function

f118c3e

Improve log messages for a bit

2368c15

relax partitionName checking to contains, instead of equals

cabffdc

kakwok added 2 commits March 6, 2018 17:13

Add containerAllFMChildren to store all LV2 FMs(Not just the active o…

112f53e

…nes)

LV1 validates Partition attribute input

1f5c793

kakwok added 2 commits March 6, 2018 18:00

Allow FM fullname as valid partition

94b311b

Do not allow Partition attribute to be used in CfgScript

dbdad74

kakwok merged commit 958801d into integration Mar 8, 2018

jhakala mentioned this pull request Mar 10, 2018

Allow partition-specific setting to be applied from FM #285

Closed

jhakala deleted the LV2readMaster branch March 16, 2018 23:13

kakwok mentioned this pull request Mar 26, 2018

Simpler mastersnippet parsing #332

Open

Lv2 reads master #363

Lv2 reads master #363

Uh oh!

Conversation

kakwok commented Feb 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kakwok Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

kakwok Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

kakwok Feb 28, 2018

Choose a reason for hiding this comment

Uh oh!

jhakala commented Mar 1, 2018

Uh oh!

kakwok commented Mar 1, 2018

Uh oh!

jhakala commented Mar 1, 2018

Uh oh!

kakwok commented Mar 1, 2018

Uh oh!

kakwok commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kakwok commented Mar 2, 2018

Uh oh!

jhakala commented Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhakala commented Mar 6, 2018

Uh oh!

kakwok commented Mar 6, 2018

Uh oh!

kakwok commented Mar 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kakwok commented Feb 28, 2018 •

edited

Loading

kakwok commented Mar 1, 2018 •

edited

Loading

jhakala commented Mar 6, 2018 •

edited

Loading