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

instructionCompiler: "Number of components" message should be toned down #743

Closed
madig opened this issue Apr 25, 2023 · 7 comments · Fixed by #766
Closed

instructionCompiler: "Number of components" message should be toned down #743

madig opened this issue Apr 25, 2023 · 7 comments · Fixed by #766

Comments

@madig
Copy link
Collaborator

madig commented Apr 25, 2023

I'm getting flooded with these messages when using the flattening filter. Maybe the warning shouldn't be fired then or made a debug message.

@jenskutilek
Copy link
Collaborator

Do the UFOs have explicit composite flags? Probably the InstructionCompiler shouldn't care about the number of components changing if it the flags are set automatically.

@anthrotype
Copy link
Member

it seems like InstructionCompiler currently issues that warning regardless of whether we are setting the flags automatically or not. I think we should only issue the warning when we are setting the flags manually (autoUseMyMetrics is not True)

anthrotype added a commit that referenced this issue Jul 19, 2023
…noisy

We should only issue this warning when USE_MY_METRICS flags are set manually from the UFO.
Fixes #743
anthrotype added a commit that referenced this issue Jul 19, 2023
…noisy

We should only issue this warning when USE_MY_METRICS flags are set manually from the UFO.
Fixes #743
@jenskutilek
Copy link
Collaborator

I think it's not only USE_MY_METRICS. The original components may have had other flags that cannot be set in the TTF when the number of components has changed.

@anthrotype
Copy link
Member

hm but the warning is issued just before calling autoUseMyMetrics on that component...
I thought it'd make sense to not issue any warning when autoUseMyMetrics=True (the default, hence most common case), but only issue when one has deliberately disabled it..
If you think the logic in #766 should be modified please send a PR, thanks!

@anthrotype
Copy link
Member

I'd also be ok to just completely remove the logging message or demote it to DEBUG

@jenskutilek
Copy link
Collaborator

+1 for removing it, I'll make a PR.

@jenskutilek
Copy link
Collaborator

Thinking about it again, I came to the conclusion that a debug message makes more sense. In case anyone wonders why the flags are set differently than expected ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants