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

Incorporate ALS fixes (IVS-186) #320

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Incorporate ALS fixes (IVS-186) #320

merged 2 commits into from
Nov 7, 2024

Conversation

civilx64
Copy link
Contributor

@civilx64 civilx64 commented Nov 4, 2024

  1. Upgrade to newer version of ifcopenshell to avoid bug in ALS016 related to clothoid spirals in imperial units.

  2. Adjust for the total length of vertical segments so that ALS017 checks are performed correctly.

(IVS-186, IVS-53)

1. Upgrade to newer version of ifcopenshell to avoid bug in ALS016 related to clothoid spirals in imperial units.

2. Adjust for the total length of vertical segments so that ALS017 checks are performed correctly.

(IVS-186, IVS-53)
Copy link
Contributor

@Ghesselink Ghesselink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Can you check whether you checked out the right commit from development? The updates made in the ALB012 branch are modified along with these commits.
  • I'm getting passed results for ALS017. Could this be that this has to do with the ifcopenshell version? I see the latest release of IfcOpenShell is 0.8.0, where do I find 0.8.1?

Edit; I've manually updated my conda environment to python3.12 and ifcopenshell commit 92b63a0, but still with the same result :(
image

context.scenario_outcome_state[len(context.gherkin_outcomes)] = {'scenario': scenario.name,
'last_step': scenario.steps[-1]}

context.scenario_outcome_state.append(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be
context.scenario_outcome_state.append((len(context.gherkin_outcomes)-1, {'scenario': context.scenario.name, 'last_step': context.scenario.steps[-1], 'instance_id': instance_id}))
See also

context.scenario_outcome_state.append((len(context.gherkin_outcomes)-1, {'scenario': context.scenario.name, 'last_step': context.scenario.steps[-1], 'instance_id': instance_id}))

Copy link
Contributor Author

@civilx64 civilx64 Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit you noted, afdc400, was applied but my merge of ALB012 stomped over it. I have cherry-picked it back to the top of the log as 8c75bee:

image

# display the correct scenario and insanity related to the gherkin outcome in the behave console & ci/cd report
context.scenario_outcome_state= []
context.instance_outcome_state = {}

context.instance_outcome_state = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context.instance_outcome_state = {}

We're not using instance_outcome_state anywhere, can be deleted.


# embed catched exceptions
# embed caught exceptions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as my English teacher from middle school doesn't see this .. oops

@Ghesselink
Copy link
Contributor

There are still some schema errors regarding the conversion. I'll also have a look
image
(e.g. for pass-als017-scenario01-imperial_continuous.ifc)

@civilx64
Copy link
Contributor Author

civilx64 commented Nov 5, 2024

I'm getting passed results for ALS017. Could this be that this has to do with the ifcopenshell version? I see the latest release of IfcOpenShell is 0.8.0, where do I find 0.8.1?

Edit; I've manually updated my conda environment to python3.12 and ifcopenshell commit 92b63a0, but still with the same result :(

Per slack discussion, you need to update the .venv on your local copy of the platform which you can do with the makefiles in validate/34be62b94106d96d00b723e3b1e2fef32effb6af

@Ghesselink
Copy link
Contributor

Per slack discussion, you need to update the .venv on your local copy of the platform which you can do with the makefiles in validate/34be62b94106d96d00b723e3b1e2fef32effb6af

Yes, indeed. I thought I was not using venv anymore as I switched to have my own conda environment for running the server, but didn't realize venv was still running for the worker

@civilx64 civilx64 merged commit 2429449 into development Nov 7, 2024
2 checks passed
@civilx64 civilx64 deleted the fix/IVS-186-ALS016 branch November 7, 2024 13:26
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 this pull request may close these issues.

2 participants