-
Notifications
You must be signed in to change notification settings - Fork 386
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
Catapult backend #1003
Catapult backend #1003
Conversation
…t still reorders backends/__init__.py incorrectly
…he HLS tool from the ProjectName which is used for the cpp file and top function name
switch from using ssh to https for submodules
Catapult backend
update pytest environment
… and test_cnn_mnist_qkeras for now
@@ -589,6 +589,7 @@ def get_ymodel_keras(keras_model, X): | |||
name = layer.name | |||
if ( | |||
hasattr(layer, 'activation') | |||
and hasattr(layer.activation, "__name__") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed? I think there were two parallel fixes for this in two pull requests, and all layer.acitvations that are of type Activation
or QActivation
should have name defined, right?
Hi Jovan,
You may be right. This was something we (Giuseppe and I) stumbled across a month or so ago.
From: Jovan Mitrevski ***@***.***>
Sent: Wednesday, April 17, 2024 6:05 PM
To: fastmachinelearning/hls4ml ***@***.***>
Cc: Burnette, David (DI SW ICS CSD CAT 2) ***@***.***>; Author ***@***.***>
Subject: Re: [fastmachinelearning/hls4ml] Catapult backend (PR #1003)
@jmitrevs commented on this pull request.
________________________________
In hls4ml/model/profiling.py<#1003 (comment)>:
@@ -589,6 +589,7 @@ def get_ymodel_keras(keras_model, X):
name = layer.name
if (
hasattr(layer, 'activation')
+ and hasattr(layer.activation, "__name__")
Is this really needed? I think there were two parallel fixes for this in two pull requests, and all layer.acitvations that are of type Activation or QActivation should have name defined, right?
-
Reply to this email directly, view it on GitHub<#1003 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIZ2SLTRKRSKTGCEJVLB2Y3Y54L3TAVCNFSM6AAAAABGMHLSFWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBXGYZDGOBZG4>.
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
|
I am thinking of closing this, unless you run into the problem again. I believe the addition of the check on the type is sufficient. It fixed the problem when I ran into it. |
Please reopen it if the problem does not go away. |
Ok. That is fine with me.
From: Jovan Mitrevski ***@***.***>
Sent: Thursday, April 18, 2024 8:03 AM
To: fastmachinelearning/hls4ml ***@***.***>
Cc: Burnette, David (DI SW ICS CSD CAT 2) ***@***.***>; Author ***@***.***>
Subject: Re: [fastmachinelearning/hls4ml] Catapult backend (PR #1003)
I am thinking of closing this, unless you run into the problem again. I believe the addition of the check on the type is sufficient. It fixed the problem when I ran into it.
-
Reply to this email directly, view it on GitHub<#1003 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIZ2SLQU2A2CIF5RAU6VRCLY57OAPAVCNFSM6AAAAABGMHLSFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRUGEZTIOBRGY>.
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
|
Description
Added safeguard for deref of non-existent attribute.
No test was added
Test Configuration:
Checklist
pre-commit
on the files I edited or added.