Exclusion of unnecessary parameters from AdvancedTreeSearchLmImageAndGlobalCacheJob hash calculation

[Marvin84]

Recently, I discovered that one terabyte of my space is occupied by different AdvancedTreeSearchLmImageAndGlobalCacheJobs that differ only by transition distortion penalty (TDP) values. However, all TDPs are part of our dynamic search procedure meaning that they are not relevant for the static search space and the global cache.

I am opening this issue to discuss the most clean way to exclude this or other parameters from the hash calculation. After a short discussion with @vieting, we think there are two different scenarios one can explore:

1. Pass two separate crps to the AdvancedTreeSearchJob, where the additional crp, if not None, is used here. Otherwise the old behavior is performed.
2. Change the hash calculation here, with two possible options, and by using a flag that keeps the default behavior

• Deleted all non-relavant parameters from the RasrConfig if a default value within RASR is defined
• Set some constant value where no default value is available

Please let me know what are your thoughts, before I prepare a PR.

[JackTemaki]

This is possibly a duplicate of #430

I am not really familiar with the crp handling of RASR, so I can not tell what the best solution is, but deleting non-relevant parameters sounds like breaking existing setups.

[JackTemaki]

@Marvin84 would #446 be a viable solution to your problem?

[Marvin84]

This is possibly a duplicate of #430

I am not really familiar with the crp handling of RASR, so I can not tell what the best solution is, but deleting non-relevant parameters sounds like breaking existing setups.

I edited the original text, of course I meant that there is a flag that conserves the default behavior

[Marvin84]

@Marvin84 would #446 be a viable solution to your problem?

The question is: why isn't it merged? I was not able to go through all the discussions there

[JackTemaki]

The question is: why isn't it merged? I was not able to go through all the discussions there

@michelwi still had open changes requested, and I assume @NeoLegends just changed his priorities as there was not enough interest from the rest to have it solved.

[NeoLegends]

Yeah, #430 points at exactly the same issue. Feel free to take over #446. I have other priorities at the moment.

[Marvin84]

@NeoLegends could you do a summary of why #446 was not merged yet and what were the open questions?

[JackTemaki]

@Marvin84 this is already answered, you can see the list of @michelwi requested changes at the end of the PR

[michelwi]

#446 would reduce the problem but not mitigate it completely: We would not duplicate the lm image which takes up most of the wasted storage, but we would still have multiple unnecessary duplicates of the global cache.

So we can leave the LmImageAndGlobalCacheJob as is (and deprecate it) but apply the same discussion points to the separate GlobalCacheJob.

[Marvin84]

we would still have multiple unnecessary duplicates of the global cache.

Exactly.

So we can leave the LmImageAndGlobalCacheJob as is (and deprecate it) but apply the same discussion points to the separate GlobalCacheJob.

Not sure if I understand what you are proposing.
So, you are saying let's have two separate jobs (LMImageJob and GlobalCacheJob) and for the GlobalCacheJob find a solution from the list of solutions I proposed above?

[michelwi]

We already have two separate jobs. I say:

1. Lets use them (this is what Add option to generate LM image and GC via two separate jobs #446 is about)
2. Find a solution for the GlobalCacheJob e.g. from the list you proposed above

[Marvin84]

I say

Yeah agreed.

We already have two separate jobs

If this is not a merged PR, then I would not say that we have them. Is it correct as @JackTemaki said, that in order to merge it we should address your last list of comments? Or is there anything changed in the meanwhile?

[michelwi]

We already have two separate jobs

If this is not a merged PR, then I would not say that we have them.

The jobs exist. They are just not used currently in the i6 standard pipelines.

Is it correct as @JackTemaki said, that in order to merge it we should address your last list of comments? Or is there anything changed in the meanwhile?

I haven't looked at it in a while, but I assume I still hold to my comments. But I can give it another look later.

[Marvin84]

But I can give it another look later.

that would be great, thank you

[michelwi]

But I can give it another look later.

Upon inspection I found all previous points (of me and others) still valid and I found even more problems that I added to the review.

[Marvin84]

Thank you, I will look into this next week.